Closed Bug 901440 Opened 11 years ago Closed 9 years ago

Add a maintenance task to remove binary annotations

Categories

(Toolkit :: Places, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: mak, Unassigned, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [good first bug][lang=js])

Attachments

(1 file)

we should remove all existing binary annotations in a maintenance task.
Whiteboard: [mentor=mak][lang=js]
Blocks: PlacesDiet
The idea is to add a task to PlacesDBUtils.jsm that removes any existing binary annotation, and add such task to the idle maintenance tasks
Whiteboard: [mentor=mak][lang=js] → [good next bug][mentor=mak][lang=js]
Mentor: mak77
Whiteboard: [good next bug][mentor=mak][lang=js] → [good next bug][lang=js]
Whiteboard: [good next bug][lang=js] → [good second bug][lang=js]
Whiteboard: [good second bug][lang=js] → [good first bug][lang=js]
Hi~ I would like to work on this issue, but I only have some basic knowledge about JavaScript. Could you tell me more about this bug? Thank you very much! :)
Hi. If you didn't yet, I suggest taking a look at these docs: https://developer.mozilla.org/en-US/docs/Introduction https://developer.mozilla.org/en-US/docs/Simple_Firefox_build https://developer.mozilla.org/en-US/docs/Mercurial_FAQ https://developer.mozilla.org/en-US/docs/Mercurial_Queues Now, let's look at what happens here. Basically some time ago we removed an API that allowed to add binary data to a page in places.sqlite database. Even if we removed the API, we never removed the data eventually created by it. The code we use to do maintenance on the database lives in PlacesDBUtils.jsm, it runs a series of "tasks" to cleanup a database. you can look into _getBoundCoherenceStatements method where there is a list of SQL statements executed to do those cleanups. Here, before "A.3" (see the code comments) we need to add a statement that will remove the binary annotations from moz_annos and moz_items_annos tables. These annotations can be recognized cause they have type = 4 (writing a SQL for this removal should be trivial, but feel free to ask if you have doubts, you can use SQLITE Manager add-on to test queries, just don't do on your default profile!) Then we need to write a test for this. The tests are in toolkit/components/places/tests/unit/test_preventive_maintenance.js you can copy the sub test A.2 and do the appropriate modifications, that is: in setup() create a fake binary annotation (type = 4) in both moz_annos and moz_items_annos(), finally in check() verify it has been removed.
Assignee: nobody → sabergeass
Status: NEW → ASSIGNED
(In reply to Marco Bonardo [::mak] from comment #3) Thank you for your trust! :) Last time when I rebuild mozilla source code I meet gcc-version problem. The system I use is Ubuntu 12.04 and gcc on it is default 4.6.3 but build mozilla need 4.8.2 or above :( I will try it again after I find way to update my gcc and I comment to let you know I had work on this issue. :)
You might look here for some useful information https://developer.mozilla.org/en-US/docs/Simple_Firefox_build/Linux_and_MacOS_build_preparation You might also ask on irc.mozilla.org on the channels #introduction or #build
(In reply to Marco Bonardo [::mak] from comment #5) Sorry for the late reply, I must say my computer is too old to compiler Mozilla source code, I had try a lot of times these days but still have problem :( So, I decide to leave this bug to other people who capable to fix that. Sorry :(
Oops.....I try it again and it works. Please forget I comment in comment #6 and I will commit my first ASAP :-)
(In reply to Marco Bonardo [::mak] from comment #3) > Here, before "A.3" (see the code comments) we need to add a statement that > will remove the binary annotations from moz_annos and moz_items_annos tables. > These annotations can be recognized cause they have type = 4 (writing a SQL > for this removal should be trivial, but feel free to ask if you have doubts, > you can use SQLITE Manager add-on to test queries, just don't do on your > default profile!) Hi~ I don't know exactly type = 4 means. In my point of view, I should add some lines in deleteObsoleteAnnos and deleteObsoleteItemsAnnos functions to delete type 4 annotations. Could you tell me some more informations about how to pick up type 4 annotations? Thanks :)
If you open places.sqlite database using Sqlite Manager add-on, and you look at the moz_items_annos table, you see there's a type column with values (1, 2, 3 and 4). This is what type = 4 means. The query to add should be something like: "DELETE FROM moz_items_annos WHERE type = 4" You should add this query after deleteObsoleteItemsAnnos, the code will look similar.
Hi Marco, There are a bunch of things on my schedule and I'm afraid I can't finish this issue. Sorry about that :( Please unassigned me from this bug. Thank you very much!
no worries, thank you for the notice. The bug now has also more information for other contributors :)
Assignee: sabergeass → nobody
Status: ASSIGNED → NEW
Priority: -- → P2
Keywords: perf
I tried to fix the bug. This is my first bug hopefully this helps.
Attachment #8697873 - Flags: review?(asaf)
Attachment #8697873 - Flags: feedback?
Attachment #8697873 - Flags: review?(mak77)
Attachment #8697873 - Flags: review?(asaf)
Attachment #8697873 - Flags: feedback?
Comment on attachment 8697873 [details] [diff] [review] Added the lines to delete type 4 Review of attachment 8697873 [details] [diff] [review]: ----------------------------------------------------------------- yep, it looks good, thanks
Attachment #8697873 - Flags: review?(mak77) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: