494952 - Form sync generates too many records

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Dan Mills [:thunder]]

This is what I get when I enable form sync:

2009-05-26 13:44:50	Engine.Forms         DEBUG	Preparing 2069 outgoing records

And I've seen logs in the forum from people who have many more items (~9K, ~14K), I don't think it's atypical.

I think our sync engine superclass is not designed for that many items.  We should find a solution (either use a different algorithm or figure out how to combine some of these records together), and probably disable form sync in the meantime (in the prefs UI).

Comment 1

[Mike Connor [:mconnor]]

So, we did add code to do expiration of form history [1], but I don't know if that's enough to mitigate this.  We do have semi-fake frecency in form history, and last used dates, and we should be able to use those to limit further if we need to.

[1] http://blog.mozilla.com/dolske/2009/05/13/happy-form-history-expiration-day/

Comment 2

[Ed Lee :Mardak]

Right now we do..
    let stmnt = this._formDB.createStatement("SELECT * FROM moz_formhistory");

We can easily sort by the form usage count and limit it.

Comment 3

[Mike Connor [:mconnor]]

Yeah, we could be that simple too.  Eventually we can co-opt something like https://bugzilla.mozilla.org/show_bug.cgi?id=370117 but count is probably sufficient for our purposes.  Or count > 3 + last 200 saved?

Comment 4

[Anant Narayanan [:anant]]

Per discussion with thunder, we're going to sync the last 200 items (one record per item, even if the field name is the same) and expire each record after a period of 90 days. Let's test and see if this improves performance and offers a reasonable user experience and make revisions as neccessary.

Comment 5

[Anant Narayanan [:anant]]

Some performance boost can be gained is if we figure out a way to generate and maintain GUIDs for each form history item. Currently, we perform a SHA1 on the field+value which guarantees GUIDness but is slow.

Is maintaining a seperate table in moz_formhistory that maps GUIDs to keys worth it?

Comment 6

[Ed Lee :Mardak]

Why lastUsed instead of timesUsed?

SELECT COUNT(*) FROM (
  SELECT * FROM moz_formhistory ORDER BY lastUsed DESC LIMIT 200)
WHERE id NOT IN (
  SELECT id FROM moz_formhistory ORDER BY timesUsed DESC LIMIT 200);

184 items in the most used/recent aren't in the other most recent/used for me.

Comment 7

[Anant Narayanan [:anant]]

Attachment: Sync only the last 200 form items

Tweak the SQL query, and remove form items older than a month.

Comment 8

[Anant Narayanan [:anant]]

(In reply to comment #6)
> Why lastUsed instead of timesUsed?

The query in the patch is:
SELECT * FROM moz_formhistory ORDER BY lastUsed DESC, timesUsed DESC LIMIT 200;

We could reverse the order of lastUsed and timesUsed -- but I think lastUsed should trump timesUsed unless timesUsed is relatively large (say >5?)

Comment 9

[Ed Lee :Mardak]

(In reply to comment #8)
> ORDER BY lastUsed DESC, timesUsed DESC LIMIT 200;
The extra timesUsed is basically a no-op... unless you're able to submit multiple forms at the very same microsecond. ;)

Comment 10

[Ed Lee :Mardak]

How about...

SELECT * FROM moz_formhistory ORDER BY 1.0 * (lastUsed - (SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed ASC LIMIT 1)) / ((SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed DESC LIMIT 1) - (SELECT lastUsed FROM moz_formhistory ORDER BY lastUsed ASC LIMIT 1)) * timesUsed / (SELECT timesUsed FROM moz_formhistory ORDER BY timesUsed DESC LIMIT 1) DESC LIMIT 200;

;)

Which is more readable form..

SELECT * FROM moz_formhistory ORDER BY 1.0 * (lastUsed - minLast) / (maxLast - minLast) * timesUsed / minTimes DESC LIMIT 200

But if not that, why not select top 100 lastUsed union select top 100 timesUsed.

Comment 11

[Anant Narayanan [:anant]]

I like both the alternate queries, and yes the timesUsed in my query was useless :-)

thunder?

Comment 12

[Anant Narayanan [:anant]]

Attachment: Sync only the last 200 (most relevant) form items

Patch with Ed's optimized query. Worked fine on my local tests.

Comment 13

[Dan Mills [:thunder]]

Comment on attachment 388118 [details] [diff] [review]
Sync only the last 200 (most relevant) form items

Looks good.  I didn't test the sql statement, I trust you that it works ;)

r=thunder

Comment 14

[Anant Narayanan [:anant]]

http://hg.mozilla.org/labs/weave/rev/5e0dcfa417f2