nettika/geary
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix a number of database hiccups

Browse source 
1) Use docid instead of id in search table.

We had previously included an 'id INTEGER PRIMARY KEY' column in the
MessageSearchTable, assuming it would get the same rowid alias treatment
as it does in non-FTS tables.  That assumption was wrong: it was being
created as a FTS column.  This fixes it so we use docid everywhere.

To fix the old incorrect docid values, we simply blow away the search
table and let the natural search table population process, which now has
the correct docid insertion code, fix the problem.

This also removes the id column from the search table creation SQL, but
this will only affect new users.  Upgraders will see an empty, vestigal
id column in their search table.  Since SQLite doesn't easily let you
remove columns, it's just easier to ignore the column than go through
all the work to fix it.

2) Do as many rowid lookups as possible in batches, instead of doing
them individually in loops.  This speeds up working with large sets of
email.

3) Rejigger indices on the MessageLocationTable to make certain queries
faster.  This creates a new covering index in particular for the email
prefetcher, which previously had to sort using a temp table.  The new
index should work in the general case too, as we should never be looking
at ordering without folder_id (and since folder_id comes first, it works
as an index on just folder_id, too).

4) For bonus measure, log all slow queries (> 1s execution time) to
debug output.

Closes: bgo #725929
This commit is contained in:
Charles Lindsay 2014-03-12 16:39:22 -07:00
parent 479f87e2ca
commit c3e5270517
10 changed files with 191 additions and 81 deletions
Download patch file Download diff file Expand all files Collapse all files

2
sql/CMakeLists.txt
View file

@ -19,3 +19,5 @@ install(FILES version-016.sql DESTINATION ${SQL_DEST})
install(FILES version-017.sql DESTINATION ${SQL_DEST})
install(FILES version-018.sql DESTINATION ${SQL_DEST})
install(FILES version-019.sql DESTINATION ${SQL_DEST})
install(FILES version-020.sql DESTINATION ${SQL_DEST})
install(FILES version-021.sql DESTINATION ${SQL_DEST})

7
sql/version-020.sql Normal file
View file

@ -0,0 +1,7 @@
--
-- We had previously incorrectly included an id column in the search table.
-- The code is fixed to use docid instead, so we just empty the table and let
-- the natural search table population process make things right.
--
DELETE FROM MessageSearchTable

13
sql/version-021.sql Normal file
View file

@ -0,0 +1,13 @@
--
-- Some queries that hit the MessageLocationTable, like those used by the email
-- prefetcher, were slow because we didn't have a covering index. This makes
-- an index that *is* covering, for the cases in question anyway. Since we
-- (should) never care about ordering without folder_id, and since folder_id
-- comes first here so this index effectively indexes queries on just that
-- field too, we can also drop the old, ineffective indices.
--
DROP INDEX MessageLocationTableFolderIdIndex;
DROP INDEX MessageLocationTableOrderingIndex;
CREATE INDEX MessageLocationTableFolderIDOrderingIndex
ON MessageLocationTable(folder_id, ordering);

3
src/engine/db/db-result.vala
View file

@ -35,7 +35,10 @@ public class Geary.Db.Result : Geary.Db.Context {
check_cancelled("Result.next", cancellable);
if (!finished) {
Timer timer = new Timer();
finished = throw_on_error("Result.next", statement.stmt.step(), statement.sql) != Sqlite.ROW;
if (timer.elapsed() > 1.0)
debug("\n\nDB QUERY STEP \"%s\"\nelapsed=%lf\n\n", statement.sql, timer.elapsed());
log(finished ? "NO ROW" : "ROW");
}

6
src/engine/imap-db/imap-db-account.vala
View file

@ -887,7 +887,7 @@ private class Geary.ImapDB.Account : BaseObject {
FROM MessageTable
INDEXED BY MessageTableInternalDateTimeTIndex
WHERE id IN (
SELECT id
SELECT docid
FROM MessageSearchTable
WHERE 1=1
""");
@ -958,7 +958,7 @@ private class Geary.ImapDB.Account : BaseObject {
sql.append("""
SELECT offsets(MessageSearchTable), *
FROM MessageSearchTable
WHERE id IN (
WHERE docid IN (
""");
sql_append_ids(sql,
Geary.traverse<ImapDB.EmailIdentifier>(ids).map<int64?>(id => id.message_id).to_gee_iterable());
@ -1126,7 +1126,7 @@ private class Geary.ImapDB.Account : BaseObject {
SELECT id
FROM MessageTable
WHERE id NOT IN (
SELECT id
SELECT docid
FROM MessageSearchTable
)
LIMIT ?

5
src/engine/imap-db/imap-db-database.vala
View file

@ -158,7 +158,6 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
// algorithm) is determined at runtime.
exec("""
CREATE VIRTUAL TABLE MessageSearchTable USING fts4(
id INTEGER PRIMARY KEY,
body,
attachment,
subject,
@ -394,8 +393,8 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase {
return Db.TransactionOutcome.COMMIT;
});
} catch (Error err) {
debug("Error populating autocompletion table during upgrade to database schema 5");
} catch (Error e) {
debug("Error fixing up contacts table: %s", e.message);
}
}

224
src/engine/imap-db/imap-db-folder.vala
View file

@ -18,7 +18,8 @@
private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
public const Geary.Email.Field REQUIRED_FIELDS = Geary.Email.Field.PROPERTIES;
private const int LIST_EMAIL_CHUNK_COUNT = 5;
private const int LIST_EMAIL_WITH_MESSAGE_CHUNK_COUNT = 10;
private const int LIST_EMAIL_METADATA_COUNT = 100;
private const int LIST_EMAIL_FIELDS_CHUNK_COUNT = 500;
[Flags]
@ -424,6 +425,12 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// database ... first, gather locations of all emails in database
Gee.List<LocationIdentifier> locations = new Gee.ArrayList<LocationIdentifier>();
yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
// convert ids into LocationIdentifiers
Gee.List<LocationIdentifier>? locs = do_get_locations_for_ids(cx, ids, flags,
cancellable);
if (locs == null || locs.size == 0)
return Db.TransactionOutcome.DONE;
StringBuilder sql = new StringBuilder("""
SELECT MessageLocationTable.message_id, ordering, remove_marker
FROM MessageLocationTable
@ -436,24 +443,25 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
""");
}
sql.append("WHERE folder_id = ? ");
if (locs.size != 1) {
sql.append("WHERE ordering IN (");
bool first = true;
foreach (LocationIdentifier location in locs) {
if (!first)
sql.append(",");
sql.append(location.uid.to_string());
first = false;
}
sql.append(")");
} else {
sql.append_printf("WHERE ordering = '%s' ", locs[0].uid.to_string());
}
if (only_incomplete)
sql.append_printf(" AND fields != %d ", Geary.Email.Field.ALL);
sql.append("AND ordering IN (");
bool first = true;
foreach (ImapDB.EmailIdentifier id in ids) {
LocationIdentifier? location = do_get_location_for_id(cx, id, flags, cancellable);
if (location == null)
continue;
if (!first)
sql.append(", ");
sql.append(location.uid.to_string());
first = false;
}
sql.append(")");
sql.append("AND folder_id = ? ");
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
@ -472,12 +480,16 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
if (ids == null || ids.size == 0)
return null;
int length_rounded_up = Numeric.int_round_up(ids.size, LIST_EMAIL_CHUNK_COUNT);
// chunk count depends on whether or not the message -- body + headers -- is being fetched
int chunk_count = required_fields.requires_any(Email.Field.BODY | Email.Field.HEADER)
? LIST_EMAIL_WITH_MESSAGE_CHUNK_COUNT : LIST_EMAIL_METADATA_COUNT;
int length_rounded_up = Numeric.int_round_up(ids.size, chunk_count);
Gee.List<Geary.Email> results = new Gee.ArrayList<Geary.Email>();
for (int start = 0; start < length_rounded_up; start += LIST_EMAIL_CHUNK_COUNT) {
for (int start = 0; start < length_rounded_up; start += chunk_count) {
// stop is the index *after* the end of the slice
int stop = Numeric.int_ceiling((start + LIST_EMAIL_CHUNK_COUNT), ids.size);
int stop = Numeric.int_ceiling((start + chunk_count), ids.size);
Gee.List<LocationIdentifier>? slice = ids.slice(start, stop);
assert(slice != null && slice.size > 0);
@ -608,9 +620,10 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// for another Folder
Gee.Set<Imap.UID> uids = new Gee.HashSet<Imap.UID>();
yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
foreach (ImapDB.EmailIdentifier id in ids) {
LocationIdentifier? location = do_get_location_for_id(cx, id, flags, cancellable);
if (location != null)
Gee.List<LocationIdentifier>? locs = do_get_locations_for_ids(cx, ids, flags,
cancellable);
if (locs != null) {
foreach (LocationIdentifier location in locs)
uids.add(location.uid);
}
@ -640,10 +653,10 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
ListFlags flags, Cancellable? cancellable) throws Error {
Gee.Set<ImapDB.EmailIdentifier> ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => {
foreach (Imap.UID uid in uids) {
LocationIdentifier? location = do_get_location_for_uid(cx, uid, flags,
cancellable);
if (location != null)
Gee.List<LocationIdentifier>? locs = do_get_locations_for_uids(cx, uids, flags,
cancellable);
if (locs != null) {
foreach (LocationIdentifier location in locs)
ids.add(location.email_id);
}
@ -692,30 +705,31 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
// (since it may be located in multiple folders). This means at some point in the future
// a vacuum will be required to remove emails that are completely unassociated with the
// account.
yield db.exec_transaction_async(Db.TransactionType.WO, (cx) => {
yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => {
Gee.List<LocationIdentifier>? locs = do_get_locations_for_ids(cx, ids,
ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
if (locs == null || locs.size == 0)
return Db.TransactionOutcome.DONE;
unread_count = do_get_unread_count_for_ids(cx, ids, cancellable);
do_add_to_unread_count(cx, -unread_count, cancellable);
// prepare DELETE Statement and invariants
Db.Statement delete_stmt = cx.prepare(
"DELETE FROM MessageLocationTable WHERE folder_id=? AND message_id=?");
delete_stmt.bind_rowid(0, folder_id);
// remove one at a time, gather UIDs
Gee.HashSet<Imap.UID> uids = new Gee.HashSet<Imap.UID>();
foreach (ImapDB.EmailIdentifier id in ids) {
LocationIdentifier? location = do_get_location_for_id(cx, id,
ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
if (location == null)
continue;
delete_stmt.reset(Db.ResetScope.SAVE_BINDINGS);
delete_stmt.bind_rowid(1, location.message_id);
delete_stmt.exec(cancellable);
uids.add(location.uid);
StringBuilder sql = new StringBuilder("""
DELETE FROM MessageLocationTable WHERE (
""");
bool first = true;
foreach (LocationIdentifier location in locs) {
if (!first)
sql.append(" OR ");
sql.append_printf(" message_id='%s' ", location.message_id.to_string());
first = false;
}
sql.append(") AND folder_id=?");
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
stmt.exec(cancellable);
return Db.TransactionOutcome.COMMIT;
}, cancellable);
@ -907,16 +921,17 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
int unread_count = 0;
Gee.Set<ImapDB.EmailIdentifier> removed_ids = new Gee.HashSet<ImapDB.EmailIdentifier>();
yield db.exec_transaction_async(Db.TransactionType.RW, (cx) => {
Gee.List<LocationIdentifier?> locs = do_get_locations_for_ids(cx, ids,
ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
if (locs == null || locs.size == 0)
return Db.TransactionOutcome.DONE;
unread_count = do_get_unread_count_for_ids(cx, ids, cancellable);
Gee.HashSet<Imap.UID> uids = new Gee.HashSet<Imap.UID>();
foreach (ImapDB.EmailIdentifier id in ids) {
LocationIdentifier? location = do_get_location_for_id(cx, id,
ListFlags.INCLUDE_MARKED_FOR_REMOVE, cancellable);
if (location != null) {
uids.add(location.uid);
removed_ids.add(location.email_id);
}
foreach (LocationIdentifier location in locs) {
uids.add(location.uid);
removed_ids.add(location.email_id);
}
if (uids.size > 0)
@ -1023,21 +1038,22 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
continue;
yield db.exec_transaction_async(Db.TransactionType.RO, (cx, cancellable) => {
Gee.List<LocationIdentifier>? locs = do_get_locations_for_ids(cx, ids, flags,
cancellable);
if (locs == null || locs.size == 0)
return Db.TransactionOutcome.DONE;
Db.Statement fetch_stmt = cx.prepare(
"SELECT fields FROM MessageTable WHERE id = ?");
foreach (ImapDB.EmailIdentifier id in list) {
LocationIdentifier? location_id = do_get_location_for_id(cx, id, flags,
cancellable);
if (location_id == null)
continue;
// TODO: Unroll loop
foreach (LocationIdentifier location in locs) {
fetch_stmt.reset(Db.ResetScope.CLEAR_BINDINGS);
fetch_stmt.bind_rowid(0, location_id.message_id);
fetch_stmt.bind_rowid(0, location.message_id);
Db.Result results = fetch_stmt.exec(cancellable);
if (!results.finished)
map.set(id, (Geary.Email.Field) results.int_at(0));
map.set(location.email_id, (Geary.Email.Field) results.int_at(0));
}
return Db.TransactionOutcome.SUCCESS;
@ -1085,6 +1101,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return !results.finished ? results.int_at(0) : 0;
}
// TODO: Unroll loop
private void do_mark_unmark_removed(Db.Connection cx, Gee.Collection<Imap.UID> uids,
bool mark_removed, Cancellable? cancellable) throws Error {
// prepare Statement for reuse
@ -1332,7 +1349,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
Db.Statement stmt = cx.prepare("""
INSERT INTO MessageSearchTable
(id, body, attachment, subject, from_field, receivers, cc, bcc)
(docid, body, attachment, subject, from_field, receivers, cc, bcc)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
""");
stmt.bind_rowid(0, message_id);
@ -1349,7 +1366,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
private static bool do_check_for_message_search_row(Db.Connection cx, int64 message_id,
Cancellable? cancellable) throws Error {
Db.Statement stmt = cx.prepare("SELECT 'TRUE' FROM MessageSearchTable WHERE id=?");
Db.Statement stmt = cx.prepare("SELECT 'TRUE' FROM MessageSearchTable WHERE docid=?");
stmt.bind_rowid(0, message_id);
Db.Result result = stmt.exec(cancellable);
@ -1491,16 +1508,18 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
private Gee.Map<ImapDB.EmailIdentifier, Geary.EmailFlags>? do_get_email_flags(Db.Connection cx,
Gee.Collection<ImapDB.EmailIdentifier> ids, Cancellable? cancellable) throws Error {
Gee.List<LocationIdentifier>? locs = do_get_locations_for_ids(cx, ids, ListFlags.NONE,
cancellable);
if (locs == null || locs.size == 0)
return null;
// prepare Statement for reuse
Db.Statement fetch_stmt = cx.prepare("SELECT flags FROM MessageTable WHERE id=?");
Gee.Map<ImapDB.EmailIdentifier, Geary.EmailFlags> map = new Gee.HashMap<
Geary.EmailIdentifier, Geary.EmailFlags>();
foreach (ImapDB.EmailIdentifier id in ids) {
LocationIdentifier? location = do_get_location_for_id(cx, id, ListFlags.NONE, cancellable);
if (location == null)
continue;
// TODO: Unroll this loop
foreach (LocationIdentifier location in locs) {
fetch_stmt.reset(Db.ResetScope.CLEAR_BINDINGS);
fetch_stmt.bind_rowid(0, location.message_id);
@ -1528,6 +1547,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return new Geary.Imap.EmailFlags(Geary.Imap.MessageFlags.deserialize(results.string_at(0)));
}
// TODO: Unroll loop
private void do_set_email_flags(Db.Connection cx, Gee.Map<ImapDB.EmailIdentifier, Geary.EmailFlags> map,
Cancellable? cancellable) throws Error {
Db.Statement update_stmt = cx.prepare(
@ -1730,7 +1750,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
}
Db.Statement stmt = cx.prepare(
"UPDATE MessageSearchTable SET body=?, attachment=?, receivers=? WHERE id=?");
"UPDATE MessageSearchTable SET body=?, attachment=?, receivers=? WHERE docid=?");
stmt.bind_string(0, body);
stmt.bind_string(1, email.get_searchable_attachment_list());
stmt.bind_string(2, recipients);
@ -1741,7 +1761,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
if (new_fields.is_any_set(Geary.Email.Field.SUBJECT)) {
Db.Statement stmt = cx.prepare(
"UPDATE MessageSearchTable SET subject=? WHERE id=?");
"UPDATE MessageSearchTable SET subject=? WHERE docid=?");
stmt.bind_string(0, (email.subject != null ? email.subject.to_searchable_string() : null));
stmt.bind_rowid(1, message_id);
@ -1750,7 +1770,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
if (new_fields.is_any_set(Geary.Email.Field.ORIGINATORS)) {
Db.Statement stmt = cx.prepare(
"UPDATE MessageSearchTable SET from_field=? WHERE id=?");
"UPDATE MessageSearchTable SET from_field=? WHERE docid=?");
stmt.bind_string(0, (email.from != null ? email.from.to_searchable_string() : null));
stmt.bind_rowid(1, message_id);
@ -1759,7 +1779,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
if (new_fields.is_any_set(Geary.Email.Field.RECEIVERS)) {
Db.Statement stmt = cx.prepare(
"UPDATE MessageSearchTable SET cc=?, bcc=? WHERE id=?");
"UPDATE MessageSearchTable SET cc=?, bcc=? WHERE docid=?");
stmt.bind_string(0, (email.cc != null ? email.cc.to_searchable_string() : null));
stmt.bind_string(1, (email.bcc != null ? email.bcc.to_searchable_string() : null));
stmt.bind_rowid(2, message_id);
@ -2049,6 +2069,36 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return (!flags.include_marked_for_remove() && location.marked_removed) ? null : location;
}
private Gee.List<LocationIdentifier>? do_get_locations_for_ids(Db.Connection cx,
Gee.Collection<ImapDB.EmailIdentifier>? ids, ListFlags flags, Cancellable? cancellable)
throws Error {
if (ids == null || ids.size == 0)
return null;
StringBuilder sql = new StringBuilder("""
SELECT message_id, ordering, remove_marker
FROM MessageLocationTable
WHERE message_id IN (
""");
bool first = true;
foreach (ImapDB.EmailIdentifier id in ids) {
if (!first)
sql.append(",");
sql.append_printf(id.message_id.to_string());
first = false;
}
sql.append(") AND folder_id = ?");
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), flags,
cancellable);
return (locs.size > 0) ? locs : null;
}
private LocationIdentifier? do_get_location_for_uid(Db.Connection cx, Imap.UID uid,
ListFlags flags, Cancellable? cancellable) throws Error {
Db.Statement stmt = cx.prepare("""
@ -2068,6 +2118,36 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
return (!flags.include_marked_for_remove() && location.marked_removed) ? null : location;
}
private Gee.List<LocationIdentifier>? do_get_locations_for_uids(Db.Connection cx,
Gee.Collection<Imap.UID>? uids, ListFlags flags, Cancellable? cancellable)
throws Error {
if (uids == null || uids.size == 0)
return null;
StringBuilder sql = new StringBuilder("""
SELECT message_id, ordering, remove_marker
FROM MessageLocationTable
WHERE ordering IN (
""");
bool first = true;
foreach (Imap.UID uid in uids) {
if (!first)
sql.append(",");
sql.append(uid.value.to_string());
first = false;
}
sql.append(") AND folder_id = ?");
Db.Statement stmt = cx.prepare(sql.str);
stmt.bind_rowid(0, folder_id);
Gee.List<LocationIdentifier> locs = do_results_to_locations(stmt.exec(cancellable), flags,
cancellable);
return (locs.size > 0) ? locs : null;
}
private int do_get_unread_count_for_ids(Db.Connection cx,
Gee.Collection<ImapDB.EmailIdentifier> ids, Cancellable? cancellable) throws Error {
// Fetch flags for each email and update this folder's unread count.

6
src/engine/imap-engine/imap-engine-account-synchronizer.vala
View file

@ -131,6 +131,12 @@ private class Geary.ImapEngine.AccountSynchronizer : Geary.BaseObject {
if (imap_folder == null)
continue;
// if considering folder not because it's available (i.e. because its contents changed),
// and the folder is open, don't process it; MinimalFolder will take care of changes as
// they occur, in order to remain synchronized
if (imap_folder.get_open_state() != Folder.OpenState.CLOSED)
continue;
// don't requeue the currently processing folder
if (imap_folder != current_folder)
bg_queue.send(imap_folder);

2
src/engine/imap-engine/imap-engine-replay-queue.vala
View file

@ -499,7 +499,7 @@ private class Geary.ImapEngine.ReplayQueue : Geary.BaseObject {
remote_err = replay_err;
}
} else if (!is_close_op) {
remote_err = new EngineError.SERVER_UNAVAILABLE("Folder %s not available", to_string());
remote_err = new EngineError.SERVER_UNAVAILABLE("Folder %s not available", owner.to_string());
}
bool has_failed = !is_close_op && (status == ReplayOperation.Status.FAILED);

2
src/engine/imap-engine/replay-ops/imap-engine-remove-email.vala
View file

@ -79,7 +79,7 @@ private class Geary.ImapEngine.RemoveEmail : Geary.ImapEngine.SendReplayOperatio
}
public override string describe_state() {
return "to_remove=%d removed_ids=%d".printf(to_remove.size,
return "to_remove.size=%d removed_ids.size=%d".printf(to_remove.size,
(removed_ids != null) ? removed_ids.size : 0);
}
}