← Back to team overview

zeitgeist team mailing list archive

[Branch ~zeitgeist/zeitgeist/bluebird] Rev 461: Enhanced database corruption detection and recovery.

 

------------------------------------------------------------
revno: 461
fixes bug: https://launchpad.net/bugs/961974
committer: Siegfried-Angel Gevatter Pujals <siegfried@xxxxxxxxxxxx>
branch nick: bluebird
timestamp: Thu 2012-04-05 14:12:49 +0200
message:
  Enhanced database corruption detection and recovery.
modified:
  extensions/histogram.vala
  src/db-reader.vala
  src/engine.vala
  src/extension-store.vala
  src/sql-schema.vala
  src/sql.vala
  src/table-lookup.vala
  src/utils.vala


--
lp:zeitgeist
https://code.launchpad.net/~zeitgeist/zeitgeist/bluebird

Your team Zeitgeist Framework Team is subscribed to branch lp:zeitgeist.
To unsubscribe from this branch go to https://code.launchpad.net/~zeitgeist/zeitgeist/bluebird/+edit-subscription
=== modified file 'extensions/histogram.vala'
--- extensions/histogram.vala	2011-12-07 12:17:29 +0000
+++ extensions/histogram.vala	2012-04-05 12:12:49 +0000
@@ -96,14 +96,8 @@
 
                 builder.add ("(xu)", t, count);
             }
-
-            if (rc != Sqlite.DONE)
-            {
-                string error_message = "Error in get_histogram_data: " +
-                    "%d, %s".printf (rc, db.errmsg ());
-                warning ("%s", error_message);
-                throw new EngineError.DATABASE_ERROR (error_message);
-            }
+            database.assert_query_success (rc, "Error in get_histogram_data",
+                Sqlite.DONE);
 
             return builder.end ();
         }

=== modified file 'src/db-reader.vala'
--- src/db-reader.vala	2012-03-19 19:56:38 +0000
+++ src/db-reader.vala	2012-04-05 12:12:49 +0000
@@ -55,13 +55,22 @@
         database.set_deletion_callback (delete_from_cache);
         db = database.database;
 
-        interpretations_table = new TableLookup (database, "interpretation");
-        manifestations_table = new TableLookup (database, "manifestation");
-        mimetypes_table = new TableLookup (database, "mimetype");
-        actors_table = new TableLookup (database, "actor");
+        try
+        {
+            interpretations_table = new TableLookup (database, "interpretation");
+            manifestations_table = new TableLookup (database, "manifestation");
+            mimetypes_table = new TableLookup (database, "mimetype");
+            actors_table = new TableLookup (database, "actor");
+        }
+        catch (EngineError err)
+        {
+            // FIXME: propagate this properly?
+            critical ("TableLookup initialization failed: %s", err.message);
+        }
     }
 
     protected Event get_event_from_row (Sqlite.Statement stmt, uint32 event_id)
+        throws EngineError
     {
         Event event = new Event ();
         event.id = event_id;
@@ -88,6 +97,7 @@
     }
 
     protected Subject get_subject_from_row (Sqlite.Statement stmt)
+        throws EngineError
     {
         Subject subject = new Subject ();
         subject.uri = stmt.column_text (EventViewRows.SUBJECT_URI);
@@ -142,11 +152,7 @@
             Subject subject = get_subject_from_row(stmt);
             event.add_subject(subject);
         }
-        if (rc != Sqlite.DONE)
-        {
-            throw new EngineError.DATABASE_ERROR ("Error: %d, %s\n",
-                rc, db.errmsg ());
-        }
+        database.assert_query_success (rc, "Error", Sqlite.DONE);
 
         // Sort events according to the sequence of event_ids
         var results = new GenericArray<Event?> ();
@@ -270,7 +276,7 @@
                 warning (error_message);
                 throw new EngineError.INVALID_ARGUMENT (error_message);
         }
-        
+
         // complete the sort rule
         bool time_asc = ResultType.is_sort_order_asc ((ResultType) result_type);
         sql += " timestamp %s".printf ((time_asc) ? "ASC" : "DESC");
@@ -306,6 +312,7 @@
             string error_message = "Error in find_event_ids: %d, %s".printf (
                 rc, db.errmsg ());
             warning (error_message);
+            database.assert_not_corrupt (rc);
             throw new EngineError.DATABASE_ERROR (error_message);
         }
 
@@ -458,14 +465,7 @@
             // for (int i=0; i<related_uris.length; i++)
             //    related_uris[i] = temp_related_uris[i];
 
-            if (rc != Sqlite.DONE)
-            {
-                string error_message =
-                    "Error in find_related_uris: %d, %s".printf (
-                    rc, db.errmsg ());
-                warning (error_message);
-                throw new EngineError.DATABASE_ERROR (error_message);
-            }
+            database.assert_query_success (rc, "Error in find_related_uris");
 
             var uri_counter = new HashTable<string, RelatedUri?>(
                 str_hash, str_equal);

=== modified file 'src/engine.vala'
--- src/engine.vala	2012-03-14 14:26:11 +0000
+++ src/engine.vala	2012-04-05 12:12:49 +0000
@@ -241,6 +241,7 @@
             if ((rc = insert_stmt.step()) != Sqlite.DONE) {
                 if (rc != Sqlite.CONSTRAINT)
                 {
+                    database.assert_not_corrupt (rc);
                     warning ("SQL error: %d, %s\n", rc, db.errmsg ());
                     return 0;
                 }
@@ -261,6 +262,7 @@
                 retrieval_stmt.bind_int64 (4, actors_table.id_for_string (event.actor));
 
                 if ((rc = retrieval_stmt.step ()) != Sqlite.ROW) {
+                    database.assert_not_corrupt (rc);
                     warning ("SQL error: %d, %s\n", rc, db.errmsg ());
                     return 0;
                 }
@@ -340,6 +342,11 @@
             if ((rc = move_stmt.step()) != Sqlite.DONE) {
                 if (rc != Sqlite.CONSTRAINT)
                 {
+                    try
+                    {
+                        database.assert_not_corrupt (rc);
+                    }
+                    catch (EngineError err) {}
                     warning ("SQL error: %d, %s\n", rc, db.errmsg ());
                 }
             }
@@ -364,7 +371,14 @@
                 event.payload.data.length);
             if ((rc = payload_insertion_stmt.step ()) != Sqlite.DONE)
                 if (rc != Sqlite.CONSTRAINT)
+                {
                     warning ("SQL error: %d, %s\n", rc, db.errmsg ());
+                    try
+                    {
+                        database.assert_not_corrupt (rc);
+                    }
+                    catch (EngineError err) { }
+                }
 
             return database.database.last_insert_rowid ();
         }

=== modified file 'src/extension-store.vala'
--- src/extension-store.vala	2012-02-02 18:57:35 +0000
+++ src/extension-store.vala	2012-04-05 12:12:49 +0000
@@ -81,13 +81,20 @@
             storage_stmt.bind_blob (3, data.get_data (), (int) data.get_size ());
 
             if ((rc = storage_stmt.step ()) != Sqlite.DONE)
+            {
+                try
+                {
+                    database.assert_not_corrupt (rc);
+                }
+                catch (EngineError err) { }
                 warning ("SQL error: %d, %s", rc, db.errmsg ());
+            }
         }
 
         /**
          * Retrieve a previously stored value.
          */
-        public Variant? retrieve(string extension, string key, VariantType format)
+        public Variant? retrieve (string extension, string key, VariantType format)
         {
             retrieval_stmt.reset ();
             retrieval_stmt.bind_text (1, extension);
@@ -97,7 +104,14 @@
             if (rc != Sqlite.ROW)
             {
                 if (rc != Sqlite.DONE)
+                {
+                    try
+                    {
+                        database.assert_not_corrupt (rc);
+                    }
+                    catch (EngineError) { }
                     warning ("SQL error: %d, %s", rc, db.errmsg ());
+                }
                 return null;
             }
 

=== modified file 'src/sql-schema.vala'
--- src/sql-schema.vala	2012-02-10 16:52:57 +0000
+++ src/sql-schema.vala	2012-04-05 12:12:49 +0000
@@ -2,8 +2,9 @@
  *
  * Copyright © 2011 Collabora Ltd.
  *             By Siegfried-Angel Gevatter Pujals <siegfried@xxxxxxxxxxxx>
- * Copyright © 2011 Canonical Ltd.
+ * Copyright © 2011-2012 Canonical Ltd.
  *             By Michal Hruby <michal.hruby@xxxxxxxxxxxxx>
+ *             By Siegfried-A. Gevatter <siegfried.gevatter@xxxxxxxxxxxxxxx>
  *
  * Based upon a Python implementation (2009-2011) by:
  *  Markus Korn <thekorn@xxxxxxx>
@@ -120,6 +121,7 @@
         }
 
         public static int get_schema_version (Sqlite.Database database)
+            throws EngineError
         {
           var sql = "SELECT version FROM schema_version WHERE schema='core'";
           int schema_version = -1;
@@ -137,9 +139,35 @@
           // will be -1 if something went wrong anyway
           debug ("schema_version is %d", schema_version);
 
+          if (schema_version < -1)
+          {
+              throw new EngineError.DATABASE_CORRUPT (
+                  "Database corruption flag is set.");
+          }
           return schema_version;
         }
 
+        public static void set_corruption_flag (Sqlite.Database database)
+            throws EngineError
+        {
+            // A schema_version value smaller than -1 indicates that
+            // database corruption has been detected.
+            int version = get_schema_version (database);
+            if (version > 0)
+                version = -version;
+            set_schema_version (database, version);
+        }
+
+        private static void set_schema_version (Sqlite.Database database,
+            int schema_version) throws EngineError
+        {
+            /* The 'ON CONFLICT REPLACE' on the PK converts INSERT to UPDATE
+             * when appriopriate */
+            var schema_sql = "INSERT INTO schema_version VALUES ('%s', %d)"
+                .printf (CORE_SCHEMA, schema_version);
+            exec_query (database, schema_sql);
+        }
+
         public static void create_schema (Sqlite.Database database)
             throws EngineError
         {
@@ -458,13 +486,7 @@
                     version INT
                 )
                 """);
-
-            /* The 'ON CONFLICT REPLACE' on the PK converts INSERT to UPDATE
-             * when appriopriate */
-            var schema_sql = "INSERT INTO schema_version VALUES ('%s', %d)"
-                .printf (CORE_SCHEMA, CORE_SCHEMA_VERSION);
-            exec_query (database, schema_sql);
-
+            set_schema_version (database, CORE_SCHEMA_VERSION);
         }
 
         /**

=== modified file 'src/sql.vala'
--- src/sql.vala	2012-03-29 17:51:33 +0000
+++ src/sql.vala	2012-04-05 12:12:49 +0000
@@ -4,6 +4,8 @@
  *             By Siegfried-Angel Gevatter Pujals <siegfried@xxxxxxxxxxxx>
  *             By Seif Lotfy <seif@xxxxxxxxx>
  * Copyright © 2011 Manish Sinha <manishsinha@xxxxxxxxxx>
+ * Copyright © 2012 Canonical Ltd.
+ *             By Siegfried-A. Gevatter <siegfried.gevatter@xxxxxxxxxxxxxxx>
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU Lesser General Public License as published by
@@ -106,7 +108,7 @@
             {
                 try
                 {
-                    // Error (like a malformed database) may not be exposed
+                    // Errors (like a malformed database) may not be exposed
                     // until we try to operate on the database.
                     if (is_read_only)
                     {
@@ -142,17 +144,7 @@
                     // 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);
-                    }
+                    Utils.retire_database ();
                     open_database (false);
                 }
                 else if (rc == Sqlite.PERM || rc == Sqlite.CANTOPEN)
@@ -329,10 +321,40 @@
                 string error_message = "%s: %d, %s".printf (
                     msg, rc, database.errmsg ());
                 warning ("%s\n", error_message);
+                assert_not_corrupt (rc);
                 throw new EngineError.DATABASE_ERROR (error_message);
             }
         }
 
+        /**
+         * Ensure `rc' isn't SQLITE_CORRUPT. If it is, schedule a database
+         * retire and Zeitgeist restart so a new database can be created,
+         * unless in read-only mode, in which case EngineError.DATABASE_ERROR
+         * will be thrown.
+         *
+         * This function should be called whenever assert_query_success isn't
+         * used.
+         *
+         * @param rc error code returned by a SQLite call
+         */
+        public void assert_not_corrupt (int rc)
+            throws EngineError
+        {
+            if (unlikely (rc == Sqlite.CORRUPT))
+            {
+                warning ("It looks like your database is corrupt: %s".printf (
+                    database.errmsg ()));
+                if (!is_read_only)
+                {
+                    // Sets a flag in the database indicating that it is
+                    // corrupt. This will trigger a database retire and
+                    // re-creation on the next startup.
+                    DatabaseSchema.set_corruption_flag (database);
+                }
+                throw new EngineError.DATABASE_CORRUPT (database.errmsg ());
+            }
+        }
+
         private void prepare_read_queries () throws EngineError
         {
             int rc;

=== modified file 'src/table-lookup.vala'
--- src/table-lookup.vala	2012-03-19 19:12:41 +0000
+++ src/table-lookup.vala	2012-04-05 12:12:49 +0000
@@ -26,7 +26,7 @@
 
     public class TableLookup : Object
     {
-
+        unowned Zeitgeist.SQLite.Database database;
         unowned Sqlite.Database db;
 
         private string table;
@@ -36,7 +36,9 @@
         private Sqlite.Statement retrieval_stmt;
 
         public TableLookup (Database database, string table_name)
+            throws EngineError
         {
+            this.database = database;
             db = database.database;
             table = table_name;
             id_to_value = new HashTable<int, string>(direct_hash, direct_equal);
@@ -52,27 +54,16 @@
                     value_to_id.insert (values[1], int.parse(values[0]));
                     return 0;
                 }, null);
-            if (rc != Sqlite.OK)
-            {
-                critical ("Can't init %s table: %d, %s\n", table,
-                    rc, db.errmsg ());
-            }
+            database.assert_query_success (rc,
+                "Can't init %s table".printf (table));
 
             sql = "INSERT INTO " + table + " (value) VALUES (?)";
             rc = db.prepare_v2 (sql, -1, out insertion_stmt);
-            if (rc != Sqlite.OK)
-            {
-                // FIXME: throw exception and propagate it up to
-                //        zeitgeist-daemon to abort with DB error?
-                critical ("SQL error: %d, %s\n", rc, db.errmsg ());
-            }
+            database.assert_query_success (rc, "Error creating insertion_stmt");
 
             sql = "SELECT value FROM " + table + " WHERE id=?";
             rc = db.prepare_v2 (sql, -1, out retrieval_stmt);
-            if (rc != Sqlite.OK)
-            {
-                critical ("SQL error: %d, %s\n", rc, db.errmsg ());
-            }
+            database.assert_query_success (rc, "Error creating retrieval_stmt");
         }
 
         /**
@@ -94,7 +85,7 @@
          * @see id_try_string
          *
          */
-        public int id_for_string (string name)
+        public int id_for_string (string name) throws EngineError
         {
             int id = value_to_id.lookup (name);
             if (id == 0)
@@ -102,10 +93,8 @@
                 int rc;
                 insertion_stmt.reset ();
                 insertion_stmt.bind_text (1, name);
-                if ((rc = insertion_stmt.step ()) != Sqlite.DONE)
-                {
-                    critical ("SQL error: %d, %s\n", rc, db.errmsg ());
-                }
+                rc = insertion_stmt.step ();
+                database.assert_query_success (rc, "Error in id_for_string");
 
                 id = (int) db.last_insert_rowid ();
 
@@ -115,7 +104,7 @@
             return id;
         }
 
-        public unowned string get_value (int id)
+        public unowned string get_value (int id) throws EngineError
         {
             // When we fetch an event, it either was already in the database
             // at the time Zeitgeist started or it was inserted later -using
@@ -138,7 +127,8 @@
                 value_to_id.insert (text, id);
                 rc = retrieval_stmt.step ();
             }
-            if (rc != Sqlite.DONE || text == null)
+            database.assert_query_success (rc, "Error in get_value");
+            if (text == null)
             {
                 critical ("Error getting data from table: %d, %s\n",
                     rc, db.errmsg ());

=== modified file 'src/utils.vala'
--- src/utils.vala	2012-03-26 15:03:10 +0000
+++ src/utils.vala	2012-04-05 12:12:49 +0000
@@ -130,10 +130,19 @@
             original.copy (destination, FileCopyFlags.OVERWRITE, null, null);
         }
 
-        public void retire_database () throws Error
+        public void retire_database () throws EngineError
         {
-            File dbfile = File.new_for_path (get_database_file_path ());
-            dbfile.set_display_name (get_database_file_retire_name ());
+            try
+            {
+                File dbfile = File.new_for_path (get_database_file_path ());
+                dbfile.set_display_name (get_database_file_retire_name ());
+            }
+            catch (Error err)
+            {
+                string message = "Could not rename database: %s".printf (
+                    err.message);
+                throw new EngineError.DATABASE_RETIRE_FAILED (message);
+            }
         }
 
         /**