Merge lp:~zeitgeist/zeitgeist/dbfails into lp:~zeitgeist/zeitgeist/bluebird
- dbfails
- Merge into bluebird
Status: | Merged |
---|---|
Merge reported by: | Siegfried Gevatter |
Merged at revision: | not available |
Proposed branch: | lp:~zeitgeist/zeitgeist/dbfails |
Merge into: | lp:~zeitgeist/zeitgeist/bluebird |
Diff against target: |
329 lines (+149/-25) 6 files modified
doc/zeitgeist-daemon.1 (+6/-0) src/errors.vala (+6/-1) src/sql-schema.vala (+13/-6) src/sql.vala (+69/-7) src/utils.vala (+18/-4) src/zeitgeist-daemon.vala (+37/-7) |
To merge this branch: | bzr merge lp:~zeitgeist/zeitgeist/dbfails |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michal Hruby (community) | Approve | ||
Siegfried Gevatter | Needs Fixing | ||
Review via email: mp+87195@code.launchpad.net |
Commit message
Description of the change
- 357. By Michal Hruby
-
Add a define to explain query plans
- 358. By Michal Hruby
- 359. By Seif Lotfy
-
Add Exclusive lock for secure writing
- 360. By Michal Hruby
-
Add real-world query samples
- 361. By Michal Hruby
-
Don't use fts when using in-memory database
- 362. By Michal Hruby
-
Don't display debug messages by default
- 363. By Michal Hruby
-
Add a way to disable extensions
- 364. By Seif Lotfy
-
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
-
Fix Reindex in FTS indexer
- 366. By Michal Hruby
-
Disable the VolumeMonitor in StorageMonitor extension to avoid weird races
- 367. By Seif Lotfy
-
optimize allocations of variants
- 368. By Seif Lotfy
-
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
-
Merge Seif's FTS changes removing database writes.
- 370. By Siegfried Gevatter
-
Automatically recover from corrupt database and be more clear
on some other errors.
Michal Hruby (mhr3) wrote : | # |
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.
Preview Diff
1 | === modified file 'doc/zeitgeist-daemon.1' | |||
2 | --- doc/zeitgeist-daemon.1 2012-01-02 19:31:15 +0000 | |||
3 | +++ doc/zeitgeist-daemon.1 2012-01-25 11:17:27 +0000 | |||
4 | @@ -87,6 +87,12 @@ | |||
5 | 87 | .TP | 87 | .TP |
6 | 88 | .B 10 | 88 | .B 10 |
7 | 89 | There is already a running Zeitgeist instance. | 89 | There is already a running Zeitgeist instance. |
8 | 90 | .TP | ||
9 | 91 | .B 21 | ||
10 | 92 | Could not access the database file. | ||
11 | 93 | .TP | ||
12 | 94 | .B 22 | ||
13 | 95 | The database file is locked. | ||
14 | 90 | 96 | ||
15 | 91 | .SH SEE ALSO | 97 | .SH SEE ALSO |
16 | 92 | \fBzeitgeist-datahub\fR, \fBgnome-activity-journal\fR | 98 | \fBzeitgeist-datahub\fR, \fBgnome-activity-journal\fR |
17 | 93 | 99 | ||
18 | === modified file 'src/errors.vala' | |||
19 | --- src/errors.vala 2011-10-20 13:32:51 +0000 | |||
20 | +++ src/errors.vala 2012-01-25 11:17:27 +0000 | |||
21 | @@ -23,10 +23,15 @@ | |||
22 | 23 | [DBus (name = "org.gnome.zeitgeist.EngineError")] | 23 | [DBus (name = "org.gnome.zeitgeist.EngineError")] |
23 | 24 | public errordomain EngineError | 24 | public errordomain EngineError |
24 | 25 | { | 25 | { |
25 | 26 | BACKUP_FAILED, | ||
26 | 27 | DATABASE_BUSY, | ||
27 | 28 | DATABASE_CANTOPEN, | ||
28 | 29 | DATABASE_CORRUPT, | ||
29 | 26 | DATABASE_ERROR, | 30 | DATABASE_ERROR, |
30 | 31 | DATABASE_RETIRE_FAILED, | ||
31 | 27 | INVALID_ARGUMENT, | 32 | INVALID_ARGUMENT, |
32 | 28 | INVALID_KEY, | 33 | INVALID_KEY, |
34 | 29 | BACKUP_FAILED, | 34 | EXISTING_INSTANCE, |
35 | 30 | } | 35 | } |
36 | 31 | 36 | ||
37 | 32 | // vala doesn't include proper headers, this fixes it | 37 | // vala doesn't include proper headers, this fixes it |
38 | 33 | 38 | ||
39 | === modified file 'src/sql-schema.vala' | |||
40 | --- src/sql-schema.vala 2011-12-31 19:15:54 +0000 | |||
41 | +++ src/sql-schema.vala 2012-01-25 11:17:27 +0000 | |||
42 | @@ -414,8 +414,8 @@ | |||
43 | 414 | } | 414 | } |
44 | 415 | 415 | ||
45 | 416 | /** | 416 | /** |
48 | 417 | * Execute the given SQL. If the query doesn't succeed, log a | 417 | * Execute the given SQL. If the query doesn't succeed, throw |
49 | 418 | * critical warning (potentially aborting the program). | 418 | * an error. |
50 | 419 | * | 419 | * |
51 | 420 | * @param database the database on which to run the query | 420 | * @param database the database on which to run the query |
52 | 421 | * @param sql the SQL query to run | 421 | * @param sql the SQL query to run |
53 | @@ -426,10 +426,17 @@ | |||
54 | 426 | int rc = database.exec (sql); | 426 | int rc = database.exec (sql); |
55 | 427 | if (rc != Sqlite.OK) | 427 | if (rc != Sqlite.OK) |
56 | 428 | { | 428 | { |
61 | 429 | const string fmt_str = "Can't create database: %d, %s\n\n" + | 429 | if (rc == Sqlite.CORRUPT) |
62 | 430 | "Unable to execute SQL:\n%s"; | 430 | { |
63 | 431 | var err_msg = fmt_str.printf (rc, database.errmsg (), sql); | 431 | throw new EngineError.DATABASE_CORRUPT (database.errmsg ()); |
64 | 432 | throw new EngineError.DATABASE_ERROR (err_msg); | 432 | } |
65 | 433 | else | ||
66 | 434 | { | ||
67 | 435 | const string fmt_str = "Can't create database: %d, %s\n\n" + | ||
68 | 436 | "Unable to execute SQL:\n%s"; | ||
69 | 437 | var err_msg = fmt_str.printf (rc, database.errmsg (), sql); | ||
70 | 438 | throw new EngineError.DATABASE_ERROR (err_msg); | ||
71 | 439 | } | ||
72 | 433 | } | 440 | } |
73 | 434 | } | 441 | } |
74 | 435 | 442 | ||
75 | 436 | 443 | ||
76 | === modified file 'src/sql.vala' | |||
77 | --- src/sql.vala 2012-01-02 19:30:51 +0000 | |||
78 | +++ src/sql.vala 2012-01-25 11:17:27 +0000 | |||
79 | @@ -67,13 +67,7 @@ | |||
80 | 67 | 67 | ||
81 | 68 | public ZeitgeistDatabase () throws EngineError | 68 | public ZeitgeistDatabase () throws EngineError |
82 | 69 | { | 69 | { |
90 | 70 | message ("Opening DB from %s", Utils.get_database_file_path ()); | 70 | open_database (true); |
84 | 71 | int rc = Sqlite.Database.open_v2 ( | ||
85 | 72 | Utils.get_database_file_path (), | ||
86 | 73 | out database); | ||
87 | 74 | assert_query_success (rc, "Can't open database"); | ||
88 | 75 | |||
89 | 76 | DatabaseSchema.ensure_schema (database); | ||
91 | 77 | 71 | ||
92 | 78 | prepare_queries (); | 72 | prepare_queries (); |
93 | 79 | 73 | ||
94 | @@ -82,6 +76,74 @@ | |||
95 | 82 | database.update_hook (update_callback); | 76 | database.update_hook (update_callback); |
96 | 83 | } | 77 | } |
97 | 84 | 78 | ||
98 | 79 | private void open_database (bool retry) | ||
99 | 80 | throws EngineError | ||
100 | 81 | { | ||
101 | 82 | int rc = Sqlite.Database.open_v2 ( | ||
102 | 83 | Utils.get_database_file_path (), | ||
103 | 84 | out database); | ||
104 | 85 | |||
105 | 86 | if (rc == Sqlite.OK) | ||
106 | 87 | { | ||
107 | 88 | try | ||
108 | 89 | { | ||
109 | 90 | // Error (like a malformed database) may not be exposed | ||
110 | 91 | // until we try to operate on the database. | ||
111 | 92 | DatabaseSchema.ensure_schema (database); | ||
112 | 93 | } | ||
113 | 94 | catch (EngineError err) | ||
114 | 95 | { | ||
115 | 96 | if (err is EngineError.DATABASE_CORRUPT && retry) | ||
116 | 97 | rc = Sqlite.CORRUPT; | ||
117 | 98 | else if (err is EngineError.DATABASE_CANTOPEN) | ||
118 | 99 | rc = Sqlite.CANTOPEN; | ||
119 | 100 | else if (err is EngineError.DATABASE_BUSY) | ||
120 | 101 | rc = Sqlite.BUSY; | ||
121 | 102 | else | ||
122 | 103 | throw err; | ||
123 | 104 | } | ||
124 | 105 | } | ||
125 | 106 | |||
126 | 107 | if (rc != Sqlite.OK) | ||
127 | 108 | { | ||
128 | 109 | if (rc == Sqlite.CORRUPT && retry) | ||
129 | 110 | { | ||
130 | 111 | // The database disk image is malformed | ||
131 | 112 | warning ("It looks like your database is corrupt. " + | ||
132 | 113 | "It will be renamed and a new one will be created."); | ||
133 | 114 | try | ||
134 | 115 | { | ||
135 | 116 | Utils.retire_database (); | ||
136 | 117 | } | ||
137 | 118 | catch (Error err) | ||
138 | 119 | { | ||
139 | 120 | string message = | ||
140 | 121 | "Could not rename database: %s".printf ( | ||
141 | 122 | err.message); | ||
142 | 123 | throw new EngineError.DATABASE_RETIRE_FAILED (message); | ||
143 | 124 | } | ||
144 | 125 | open_database (false); | ||
145 | 126 | } | ||
146 | 127 | else if (rc == Sqlite.PERM || rc == Sqlite.CANTOPEN) | ||
147 | 128 | { | ||
148 | 129 | // Access permission denied / Unable to open database file | ||
149 | 130 | throw new EngineError.DATABASE_CANTOPEN ( | ||
150 | 131 | database.errmsg ()); | ||
151 | 132 | } | ||
152 | 133 | else if (rc == Sqlite.BUSY) | ||
153 | 134 | { | ||
154 | 135 | // The database file is locked | ||
155 | 136 | throw new EngineError.DATABASE_BUSY (database.errmsg ()); | ||
156 | 137 | } | ||
157 | 138 | else | ||
158 | 139 | { | ||
159 | 140 | string message = "Can't open database: %d, %s".printf(rc, | ||
160 | 141 | database.errmsg ()); | ||
161 | 142 | throw new EngineError.DATABASE_ERROR (message); | ||
162 | 143 | } | ||
163 | 144 | } | ||
164 | 145 | } | ||
165 | 146 | |||
166 | 85 | public uint32 get_last_id () throws EngineError | 147 | public uint32 get_last_id () throws EngineError |
167 | 86 | { | 148 | { |
168 | 87 | int last_id = -1; | 149 | int last_id = -1; |
169 | 88 | 150 | ||
170 | === modified file 'src/utils.vala' | |||
171 | --- src/utils.vala 2011-10-31 15:28:09 +0000 | |||
172 | +++ src/utils.vala 2012-01-25 11:17:27 +0000 | |||
173 | @@ -31,7 +31,8 @@ | |||
174 | 31 | private static string DATABASE_FILE_BACKUP_PATH; | 31 | private static string DATABASE_FILE_BACKUP_PATH; |
175 | 32 | private static string LOCAL_EXTENSIONS_PATH; | 32 | private static string LOCAL_EXTENSIONS_PATH; |
176 | 33 | 33 | ||
178 | 34 | public const string ZEITGEIST_DATA_FOLDER = "zeitgeist"; | 34 | public const string DATA_FOLDER = "zeitgeist"; |
179 | 35 | public const string DATABASE_BASENAME = "activity.sqlite"; | ||
180 | 35 | public const string USER_EXTENSION_PATH = ""; | 36 | public const string USER_EXTENSION_PATH = ""; |
181 | 36 | 37 | ||
182 | 37 | // D-Bus | 38 | // D-Bus |
183 | @@ -48,7 +49,7 @@ | |||
184 | 48 | 49 | ||
185 | 49 | DATA_PATH = Environment.get_variable ("ZEITGEIST_DATA_PATH") ?? | 50 | DATA_PATH = Environment.get_variable ("ZEITGEIST_DATA_PATH") ?? |
186 | 50 | Path.build_filename (Environment.get_user_data_dir (), | 51 | Path.build_filename (Environment.get_user_data_dir (), |
188 | 51 | ZEITGEIST_DATA_FOLDER); | 52 | DATA_FOLDER); |
189 | 52 | 53 | ||
190 | 53 | if (!FileUtils.test (DATA_PATH, FileTest.IS_DIR)) | 54 | if (!FileUtils.test (DATA_PATH, FileTest.IS_DIR)) |
191 | 54 | { | 55 | { |
192 | @@ -66,7 +67,7 @@ | |||
193 | 66 | 67 | ||
194 | 67 | DATABASE_FILE_PATH = | 68 | DATABASE_FILE_PATH = |
195 | 68 | Environment.get_variable ("ZEITGEIST_DATABASE_PATH") ?? | 69 | Environment.get_variable ("ZEITGEIST_DATABASE_PATH") ?? |
197 | 69 | Path.build_filename (get_data_path (), "activity.sqlite"); | 70 | Path.build_filename (get_data_path (), DATABASE_BASENAME); |
198 | 70 | 71 | ||
199 | 71 | debug ("DATABASE_FILE_PATH = %s", DATABASE_FILE_PATH); | 72 | debug ("DATABASE_FILE_PATH = %s", DATABASE_FILE_PATH); |
200 | 72 | 73 | ||
201 | @@ -80,13 +81,20 @@ | |||
202 | 80 | 81 | ||
203 | 81 | DATABASE_FILE_BACKUP_PATH = | 82 | DATABASE_FILE_BACKUP_PATH = |
204 | 82 | Environment.get_variable ("ZEITGEIST_DATABASE_BACKUP_PATH") ?? | 83 | Environment.get_variable ("ZEITGEIST_DATABASE_BACKUP_PATH") ?? |
206 | 83 | Path.build_filename (get_data_path (), "activity.sqlite.bck"); | 84 | Path.build_filename (get_data_path (), |
207 | 85 | DATABASE_BASENAME + ".bck"); | ||
208 | 84 | 86 | ||
209 | 85 | debug ("DATABASE_FILE_BACKUP_PATH = %s", DATABASE_FILE_BACKUP_PATH); | 87 | debug ("DATABASE_FILE_BACKUP_PATH = %s", DATABASE_FILE_BACKUP_PATH); |
210 | 86 | 88 | ||
211 | 87 | return DATABASE_FILE_BACKUP_PATH; | 89 | return DATABASE_FILE_BACKUP_PATH; |
212 | 88 | } | 90 | } |
213 | 89 | 91 | ||
214 | 92 | public string get_database_file_retire_name () | ||
215 | 93 | { | ||
216 | 94 | return DATABASE_BASENAME + ".%s.bck".printf ( | ||
217 | 95 | new DateTime.now_local ().format ("%Y%m%d-%H%M%S")); | ||
218 | 96 | } | ||
219 | 97 | |||
220 | 90 | public unowned string get_local_extensions_path () | 98 | public unowned string get_local_extensions_path () |
221 | 91 | { | 99 | { |
222 | 92 | if (LOCAL_EXTENSIONS_PATH != null) return LOCAL_EXTENSIONS_PATH; | 100 | if (LOCAL_EXTENSIONS_PATH != null) return LOCAL_EXTENSIONS_PATH; |
223 | @@ -113,6 +121,12 @@ | |||
224 | 113 | 121 | ||
225 | 114 | original.copy (destination, FileCopyFlags.OVERWRITE, null, null); | 122 | original.copy (destination, FileCopyFlags.OVERWRITE, null, null); |
226 | 115 | } | 123 | } |
227 | 124 | |||
228 | 125 | public void retire_database () throws Error | ||
229 | 126 | { | ||
230 | 127 | File dbfile = File.new_for_path (get_database_file_path ()); | ||
231 | 128 | dbfile.set_display_name (get_database_file_retire_name ()); | ||
232 | 129 | } | ||
233 | 116 | } | 130 | } |
234 | 117 | } | 131 | } |
235 | 118 | 132 | ||
236 | 119 | 133 | ||
237 | === modified file 'src/zeitgeist-daemon.vala' | |||
238 | --- src/zeitgeist-daemon.vala 2012-01-02 19:30:51 +0000 | |||
239 | +++ src/zeitgeist-daemon.vala 2012-01-25 11:17:27 +0000 | |||
240 | @@ -329,6 +329,7 @@ | |||
241 | 329 | } | 329 | } |
242 | 330 | 330 | ||
243 | 331 | static void run () | 331 | static void run () |
244 | 332 | throws Error | ||
245 | 332 | { | 333 | { |
246 | 333 | DBusConnection connection; | 334 | DBusConnection connection; |
247 | 334 | bool name_owned; | 335 | bool name_owned; |
248 | @@ -342,8 +343,7 @@ | |||
249 | 342 | } | 343 | } |
250 | 343 | catch (IOError err) | 344 | catch (IOError err) |
251 | 344 | { | 345 | { |
254 | 345 | critical ("%s", err.message); | 346 | throw err; |
253 | 346 | return; | ||
255 | 347 | } | 347 | } |
256 | 348 | if (name_owned) | 348 | if (name_owned) |
257 | 349 | { | 349 | { |
258 | @@ -353,9 +353,10 @@ | |||
259 | 353 | } | 353 | } |
260 | 354 | else | 354 | else |
261 | 355 | { | 355 | { |
263 | 356 | critical ("An existing instance was found. Please use " + | 356 | warning ("An existing instance was found. Please use " + |
264 | 357 | "--replace to stop it and start a new instance."); | 357 | "--replace to stop it and start a new instance."); |
266 | 358 | Posix.exit (10); | 358 | throw new EngineError.EXISTING_INSTANCE ( |
267 | 359 | "Zeitgeist is running already."); | ||
268 | 359 | } | 360 | } |
269 | 360 | } | 361 | } |
270 | 361 | 362 | ||
271 | @@ -370,8 +371,24 @@ | |||
272 | 370 | } | 371 | } |
273 | 371 | catch (Error err) | 372 | catch (Error err) |
274 | 372 | { | 373 | { |
277 | 373 | critical ("%s", err.message); | 374 | if (err is EngineError.DATABASE_CANTOPEN) |
278 | 374 | return; | 375 | { |
279 | 376 | warning ("Could not access the database file.\n" + | ||
280 | 377 | "Please check the permissions of file %s.", | ||
281 | 378 | Utils.get_database_file_path ()); | ||
282 | 379 | throw err; | ||
283 | 380 | } | ||
284 | 381 | else if (err is EngineError.DATABASE_BUSY) | ||
285 | 382 | { | ||
286 | 383 | warning ("It looks like another Zeitgeist instance " + | ||
287 | 384 | "is already running (the database is locked). " + | ||
288 | 385 | "If you want to start a new instance, use --replace."); | ||
289 | 386 | throw err; | ||
290 | 387 | } | ||
291 | 388 | else | ||
292 | 389 | { | ||
293 | 390 | throw err; | ||
294 | 391 | } | ||
295 | 375 | } | 392 | } |
296 | 376 | 393 | ||
297 | 377 | uint owner_id = Bus.own_name_on_connection (connection, | 394 | uint owner_id = Bus.own_name_on_connection (connection, |
298 | @@ -437,7 +454,7 @@ | |||
299 | 437 | 454 | ||
300 | 438 | return 0; | 455 | return 0; |
301 | 439 | } | 456 | } |
303 | 440 | 457 | ||
304 | 441 | LogLevelFlags discarded = LogLevelFlags.LEVEL_DEBUG; | 458 | LogLevelFlags discarded = LogLevelFlags.LEVEL_DEBUG; |
305 | 442 | if (log_level != null) | 459 | if (log_level != null) |
306 | 443 | { | 460 | { |
307 | @@ -472,9 +489,22 @@ | |||
308 | 472 | 489 | ||
309 | 473 | run (); | 490 | run (); |
310 | 474 | } | 491 | } |
311 | 492 | catch (EngineError.EXISTING_INSTANCE err) | ||
312 | 493 | { | ||
313 | 494 | return 10; | ||
314 | 495 | } | ||
315 | 496 | catch (EngineError.DATABASE_CANTOPEN err) | ||
316 | 497 | { | ||
317 | 498 | return 21; | ||
318 | 499 | } | ||
319 | 500 | catch (EngineError.DATABASE_BUSY err) | ||
320 | 501 | { | ||
321 | 502 | return 22; | ||
322 | 503 | } | ||
323 | 475 | catch (Error err) | 504 | catch (Error err) |
324 | 476 | { | 505 | { |
325 | 477 | warning ("%s", err.message); | 506 | warning ("%s", err.message); |
326 | 507 | return 1; | ||
327 | 478 | } | 508 | } |
328 | 479 | 509 | ||
329 | 480 | return 0; | 510 | return 0; |
<mhr3> RainCT, can you propagate the error one more level up, so there's no need for the posix.exit?