583852 - Weave should not be querying on places views

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Shawn Wilsher :sdwilsh]

Views are very slow to select on because they have no indexes.  Views also consume more memory (table has to be created in memory to query from).  Additionally, views are very likely to be going away in bug 552023.

Comment 1

[Philipp von Weitershausen [:philikon]]

Instead of querying the views we should query the UNION of table + temp table as per https://wiki.mozilla.org/Places/Places_SQL_queries_best_practices. This should also prevent the symptoms of bug 575423. If the temp table doesn't exist, bug 552023 landed and we don't have to worry about this anymore and query the table directly.

Comment 3

[Philipp von Weitershausen [:philikon]]

Attachment: tests

Tests for the history store.

Comment 4

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Query moz_places, moz_historyvisits *and* moz_places_temp, moz_historyvisits_temp instead of the slower views. If the temp tables aren't available (bug 552023), just query the regular tables.

Also fixed some style and syntax nits.

Comment 5

[Philipp von Weitershausen [:philikon]]

Attachment: v1.1

Harmonize spelling of table + temp table queries with places:
* query moz_*_temp before moz_*
* use IFNULL rather than IN operator in WHERE clause (potential speedup)
* use UNION ALL instead of UNION in _urlStm() and add LIMIT 1 since there's at most only 1 result row (potential speedup)

Comment 6

[Philipp von Weitershausen [:philikon]]

Attachment: tests v1.1

Make tests work when running as part of xpcshell-tests on mozilla-central. Was missing the UHist directory alias again, moved it into head_helpers.js so that all tests will have it available.

Comment 7

[Philipp von Weitershausen [:philikon]]

1.4.x:
http://hg.mozilla.org/services/fx-sync/rev/7f4053acded9
http://hg.mozilla.org/services/fx-sync/rev/7deb15c50eaf

default:
http://hg.mozilla.org/services/fx-sync/rev/dbc4c91f1344
http://hg.mozilla.org/services/fx-sync/rev/e482668c5fca

Comment 8

[Tony Chung [:tchung]]

can anyone shed some light on how this can be verified manually?   Otherwise, i just assume from the bug that it sounds like unit tests here that fail against m-c will suffice when sounding the alarm.

Comment 9

[Philipp von Weitershausen [:philikon]]

The decision to query views was made to fix bug 575423. However, querying views is slow and they're going to go away with bug 552023 (hasn't permanently landed on m-c yet, but there's there's mak's tryserver build I used to verify and run my unit tests against: http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/[e-mail]-6bf09cf997cb/tryserver-macosx/)

So essentially this bug is an alternate (better performing) way to fix bug 575423 and at the same time it prepares us for nightlies once bug 552023 lands.

Comment 10

[Tony Chung [:tchung]]

Tarek, can you please verify if this is fixed with the 1.4.4 extension?

Comment 11

[Tony Chung [:tchung]]

(In reply to comment #10)
> Tarek, can you please verify if this is fixed with the 1.4.4 extension?

sorry, wrong bug comment.  disregard.

Comment 12

[Brian Crowder]

Running Fennec with Weave, while syncing, I am seeing warnings of this type:

WARNING: 2 sort operations have occurred for the SQL statement '0xac05b118'.  See https://developer.mozilla.org/En/Storage/Warnings details.: file /home/crowder/mobilla/storage/src/mozStoragePrivateHelpers.cpp, line 138


Executing this statement:

(gdb) call sqlite3_sql(mStatements[i].mStatement)
[Thread 0xb02fdb70 (LWP 13115) exited]
$8 = 0xaba1bd68 "SELECT visit_type type, visit_date date FROM moz_historyvisits_temp WHERE place_id = IFNULL( (SELECT id FROM moz_places_temp WHERE url = :url), (SELECT id FROM moz_places WHERE url = :url) ) UNION SEL"...


I'm wondering how seriously I should take this warning, but also how we're getting behavior that according to the comment (http://hg.mozilla.org/mozilla-central/annotate/97949d240701/services/sync/modules/engines/history.js#l143) should only happen in Gecko < 2.0