600980 - Set journal_mode = WAL for dirty places profiles

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Shawn Wilsher :sdwilsh]

Similar to bug 572559, when bug 573492 lands we'll want to make sure WAL is turned on for the databases (it's a one-time operation).  This does mean the python scripts would need to use SQLite 3.7.0 or later.

Comment 1

[alice nodelman [:alice] [:anode]]

Can you give me an idea of what needs to be fun here - is it something in the db itself or a perf that needs setting?

Comment 2

[alice nodelman [:alice] [:anode]]

Um, done, not fun.  Though it should also be fun.

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #1)
> Can you give me an idea of what needs to be fun here - is it something in the
> db itself or a perf that needs setting?
WAL journal mode persists once you set it, so all we need is a one-time run of this command on the database:
PRAGMA journal_mode = WAL;

You can verify that it worked by executing this command:
PRAGMA journal_mode;
...which should return "WAL" at that time.

Comment 4

[alice nodelman [:alice] [:anode]]

Sounds like something that should also be part of profile generation script - in case we totally start over at some point we wouldn't want to lose this setting.

Comment 5

[Shawn Wilsher :sdwilsh]

(In reply to comment #4)
> Sounds like something that should also be part of profile generation script -
> in case we totally start over at some point we wouldn't want to lose this
> setting.
Yeah, that'd be ideal actually (this applies to all of our SQLite dbs if we make any other ones FWIW).

Comment 6

[Shawn Wilsher :sdwilsh]

(In reply to comment #5)
> Yeah, that'd be ideal actually (this applies to all of our SQLite dbs if we
> make any other ones FWIW).
Er, sorry, that's page size which is a different bug.  Disregard the stuff in ( )!

Comment 7

[alice nodelman [:alice] [:anode]]

Just as a note to myself, this is currently blocked on bug 573492 and will not require any action until that bug is fixed.

Comment 8

[Shawn Wilsher :sdwilsh]

(In reply to comment #7)
> Just as a note to myself, this is currently blocked on bug 573492 and will not
> require any action until that bug is fixed.
I'm guessing we probably can't do this just for the places branch, can we?

Comment 9

[alice nodelman [:alice] [:anode]]

We use the same test dbs across all branches.

Comment 10

[alice nodelman [:alice] [:anode]]

Are we now ready to make this alteration to the dirty profiles used for testing?

Comment 11

[Shawn Wilsher :sdwilsh]

(In reply to comment #10)
> Are we now ready to make this alteration to the dirty profiles used for
> testing?
Yes, but these databases would fail on 3.5 (although, I don't think we care about running the dirty tests there anymore).

Comment 12

[alice nodelman [:alice] [:anode]]

So we'd want to disable dirty tests on 3.5 and then enable this change... who should I look to to get r+ on disabling that test?

Comment 13

[Shawn Wilsher :sdwilsh]

(In reply to comment #12)
> So we'd want to disable dirty tests on 3.5 and then enable this change... who
> should I look to to get r+ on disabling that test?
I'm not sure who officially that would be.  Since that test suite is hitting places code, would a places peer/owner do?

Comment 14

[Dietrich Ayala (:dietrich)]

r+ on disabling dirty places testing on 3.5.

xoxo,

Places Module Owner

Comment 15

[alice nodelman [:alice] [:anode]]

Attachment: [checked in]disable dirty tests on firefox3.5

Disabling on firefox3.5 dirty profile tests r+ed by dietrich.

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 523351 [details] [diff] [review]
[checked in]disable dirty tests on firefox3.5

r=me

Comment 17

[Shawn Wilsher :sdwilsh]

Is this going to land in the next talos update?

Comment 18

[alice nodelman [:alice] [:anode]]

At the moment I have this lined up behind tpaint (bug 646514) which does not currently have a downtime scheduled.

If that continues to be true I'll get this rolled into the tpaint change and try and get everything landed at once.

Comment 19

[alice nodelman [:alice] [:anode]]

Comment on attachment 523351 [details] [diff] [review]
[checked in]disable dirty tests on firefox3.5

changeset:   3958:40c2e70db8ef

Comment 20

[alice nodelman [:alice] [:anode]]

Dirty tests now disabled on firefox3.5.

Comment 21

[alice nodelman [:alice] [:anode]]

Scheduled to make change on Friday April 29th at 3pm PST.

Comment 22

[alice nodelman [:alice] [:anode]]

Turns out that I no longer have access to the vm that hosts the dirty profiles,
the work will have to be done by someone with releng access.

Comment 23

[Mike Taylor [:bear]]

Alice,

Where and what file do I need to change to make this bug happen?  Can you give me a filename and sequence of steps?

Comment 24

[alice nodelman [:alice] [:anode]]

Requested info provided in email.

Comment 25

[alice nodelman [:alice] [:anode]]

Any ETA here?

Comment 26

[Dustin J. Mitchell [:dustin] (he/him)]

I'll work on this.

Comment 27

[Dustin J. Mitchell [:dustin] (he/him)]

This change is scheduled to land this Sunday, along with bug 649123 - that will mean new performance numbers on Monday morning.

Comment 28

[Dustin J. Mitchell [:dustin] (he/him)]

I found sqlite3-3.7.3.bin in ~anodelman, and since the system sqlite3 is 3.6.3, I used the former.

However:

[anodelman@cruncher public_html]$ (/usr/local/bin/python /home/anodelman/public_html/updateGenerated.py) >> /home/anodelman/public_html/reporter.log && mv /home/anodelman/public_html/dirtyDBs.zip /var/www/html/talos_profiles/. &&  mv /home/anodelman/public_html/dirtyMaxDBs.zip /var/www/html/talos_profiles/. && cp /var/www/html/talos_profiles/dirtyMaxDBs.zip /home/anodelman/public_html/. && cp /var/www/html/talos_profiles/dirtyDBs.zip /home/anodelman/public_html/.
Traceback (most recent call last):
  File "/home/anodelman/generator/places/builddb/increment_dates.py", line 49, in <module>
    main()
  File "/home/anodelman/generator/places/builddb/increment_dates.py", line 45, in main
    d.increment_dates()
  File "/home/anodelman/tools/buildfarm/utils/generator/places/builddb/generate.py", line 146, in increment_dates
    last_update_seconds = ((self.last_update() / 1000) / 1000)
  File "/home/anodelman/tools/buildfarm/utils/generator/places/builddb/generate.py", line 135, in last_update
    id = self.place_id()
  File "/home/anodelman/tools/buildfarm/utils/generator/places/builddb/generate.py", line 127, in place_id
    rows = cursor.execute(sql, [])
  File "/usr/local/lib/python2.5/site-packages/django/db/backends/util.py", line 19, in execute
    return self.cursor.execute(sql, params)
  File "/usr/local/lib/python2.5/site-packages/django/db/backends/sqlite3/base.py", line 168, in execute
    return Database.Cursor.execute(self, query, params)
sqlite3.DatabaseError: file is encrypted or is not a database

which is to say that the Python bindings are not using a new enough version of sqlite.

I'm pulling the ripcord on this upgrade for now.  We'll need to revisit later, after figuring out how to get a newer version of Python set up.

Comment 29

[Shawn Wilsher :sdwilsh]

(In reply to comment #28)
> I'm pulling the ripcord on this upgrade for now.  We'll need to revisit later,
> after figuring out how to get a newer version of Python set up.
What about just using the SQLite command line tools?

Comment 30

[Dustin J. Mitchell [:dustin] (he/him)]

Works for me.  I'd like to get this out of ~anodelman, too.

Comment 31

[Dustin J. Mitchell [:dustin] (he/him)]

So I'm happy to grant access to this system, or just provide a tarball of how it's set up now, and from there figure out how to deploy it in a self-contained fashion.  I'm not the one to be rewriting the scripts to use the command-line tool.

Comment 32

[alice nodelman [:alice] [:anode]]

If you could get me a tarball of that home directory I can try and have a look at it, see if I can reproduce the failure and then try and fix it.

Comment 33

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Clearing whiteboard after today's attempted rollout. Please renominate when you think this is ready for rollout-to-production.

Comment 34

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #34)
> This does mean the python scripts would need to use SQLite 3.7.0 or later.

(In reply to comment #28)
> I found sqlite3-3.7.3.bin in ~anodelman, and since the system sqlite3 is
> 3.6.3...

Can this work be done with sqllite3.6.3?

Comment 35

[Dustin J. Mitchell [:dustin] (he/him)]

That should be 3.3.6 (typo, sorry), but no, 3.3.6 and 3.6.3 are both before 3.7.0, which is what's required.

Comment 36

[alice nodelman [:alice] [:anode]]

I'm grabbing a copy of the working directory and will try and come up with a solution for the sqlite version problem.

Comment 37

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #35)
> That should be 3.3.6 (typo, sorry), but no, 3.3.6 and 3.6.3 are both before
> 3.7.0, which is what's required.

(In reply to comment #36)
> I'm grabbing a copy of the working directory and will try and come up with a
> solution for the sqlite version problem.

Cool. Moving to Testing:General. Feel free to bring back to RelEng when this is resolved and you're ready to try in production again.

Comment 38

[alice nodelman [:alice] [:anode]]

Okay, this took a little sorting out but here's what needs to happen:

- update sqlite3 on cruncher (I updated to 3.7.6.3), I believe that we can just update the default install as nothing else there is dependent on it
- re-install django on cruncher (this way it picks up the new sqlite3 installation) (Django-1.3), no need for upgrade here a re-install is fine
- set the pragma for the places.sqlite for each dirty profile using the new sqlite3 installation
- run the updateGenerated.py script to ensure that everything is working correctly

Following these steps I was able to do the alteration to the dirty places profiles and have the updates still work correctly.

Comment 39

[Dustin J. Mitchell [:dustin] (he/him)]

Back to me.  I'll make sure this will work on cruncher, then pass to joduinn for platform approval, then schedule the downtime, then make the change.  Easy peasy.

I'll also try to document this while I'm at it.

Comment 40

[Dustin J. Mitchell [:dustin] (he/him)]

I wrote up some docs here:
  https://wiki.mozilla.org/ReleaseEngineering/Applications/Talos_Dirty_Profiles

Comment 41

[Dustin J. Mitchell [:dustin] (he/him)]

On cruncher:

[root@cruncher ~]# yum install sqlite
Loaded plugins: rhnplugin, security
Setting up Install Process
Package sqlite-3.3.6-5.i386 already installed and latest version
Nothing to do

So I need to invent some well-isolated way of installing a newer version of sqlite here.  I *really* don't want that to involve compiling sqlite, but if that's what's required, at least I'll do it in a script..

Comment 42

[Dustin J. Mitchell [:dustin] (he/him)]

I haven't worked on this any more in the last few days.  I'm starting to think about a script that can install a specific version of sqlite3 as well as the various python prerequisites in a virtualenv - then the system would be exactly reproducible regardless of platform.

Comment 43

[Dustin J. Mitchell [:dustin] (he/him)]

Reassigning to releng

Comment 44

[alice nodelman [:alice] [:anode]]

The steps to success are still in comment #38, love to get a new releng person assigned to roll this out.

Comment 45

[Shawn Wilsher :sdwilsh]

Do we have an ETA on this by chance?

Comment 46

[Chris Cooper [:coop] (he/him)]

(In reply to alice nodelman [:alice] [:anode] from comment #38) 
> - update sqlite3 on cruncher (I updated to 3.7.6.3), I believe that we can
> just update the default install as nothing else there is dependent on it
> - re-install django on cruncher (this way it picks up the new sqlite3
> installation) (Django-1.3), no need for upgrade here a re-install is fine
> - set the pragma for the places.sqlite for each dirty profile using the new
> sqlite3 installation
> - run the updateGenerated.py script to ensure that everything is working
> correctly

I have an rpm built of sqlite-3.7.8 that I'm ready to install.

How was django originally installed on cruncher? Not as an rpm AFAICT, and I'd like to follow the same path for the upgrade (if at all possible) or switch to an rpm solution so we could revert easily if necessary.

(In reply to Shawn Wilsher :sdwilsh from comment #45)
> Do we have an ETA on this by chance?

I'm aiming to make this change during the downtime for the Aurora merge next week (Tue Sep 27th).

Comment 47

[alice nodelman [:alice] [:anode]]

The django installation was from a tar.gz (https://www.djangoproject.com/download/) in the anodelman home directory (and there may still be a copy in there), then using the 'python setup.py install'.  Last I looked at this I was using 1.3, but it appears that they are now on 1.3.1.

Comment 48

[Chris Cooper [:coop] (he/him)]

(In reply to alice nodelman [:alice] [:anode] from comment #47)
> The django installation was from a tar.gz
> (https://www.djangoproject.com/download/) in the anodelman home directory
> (and there may still be a copy in there), then using the 'python setup.py
> install'.  Last I looked at this I was using 1.3, but it appears that they
> are now on 1.3.1.

OK, thanks Alice.

In terms of deployment, once sqlite and django are updated, do I just have to open up the zips, run the PRAGMA commands, and then zip them back up again?

Comment 49

[alice nodelman [:alice] [:anode]]

Yes, just run the updateGenerated command (which should be in a crontab, so you can steal it from there) to ensure that the dbs get updated correctly.  If something goes wrong you'll see it when you run that script.

You will probably want to make back-ups of the dbs before making the changeover.  While these upgrades were successful in my testing, that was months ago now.

Comment 50

[Chris Cooper [:coop] (he/him)]

(In reply to alice nodelman [:alice] [:anode] from comment #49)
> Yes, just run the updateGenerated command (which should be in a crontab, so
> you can steal it from there) to ensure that the dbs get updated correctly. 
> If something goes wrong you'll see it when you run that script.
> 
> You will probably want to make back-ups of the dbs before making the
> changeover.  While these upgrades were successful in my testing, that was
> months ago now.

To address Dustin's conecrns, I've created a new talos user and group  on cruncher and added all the releng people (and anodelman) to the talos group as well. I've moved the public_html dir from under ~anodelman/ to ~talos/ and changed the relevant crontab entryas well. 

There are copies of the original databases under ~talos/public_html. They are marked as *.preWAL.

I ran the PRAGMA commands against the dbs, and then the updateGenerated command from the crontab under the new talos user. This completed successfully.

Keeping this open is in case there is some performance wobble.

Comment 51

[alice nodelman [:alice] [:anode]]

Sounds great, coop.  Please just double check (since paths have now changed) that 

a) the dirty profiles are updated nightly
b) the updated dirty profiles are copied to the build server

Comment 52

[Chris Cooper [:coop] (he/him)]

(In reply to alice nodelman [:alice] [:anode] from comment #51)
> Sounds great, coop.  Please just double check (since paths have now changed)
> that 
> 
> a) the dirty profiles are updated nightly
> b) the updated dirty profiles are copied to the build server

Both of these are performed by the crontab entry, correct?

I copied the crontab entry over from anodelman (and then disable it for anodelman) and changed the paths where necessary. This is the new talos user's crontab:

0 0 * * * (/usr/local/bin/python /home/talos/public_html/updateGenerated.py) >> /home/talos/public_html/reporter.log && mv /home/talos/public_html/dirtyDBs.zip /var/www/html/talos_profiles/. &&  mv /home/talos/public_html/dirtyMaxDBs.zip /var/www/html/talos_profiles/. && cp /var/www/html/talos_profiles/dirtyMaxDBs.zip /home/talos/public_html/. && cp /var/www/html/talos_profiles/dirtyDBs.zip /home/talos/public_html/.

This is also the command I ran today after setting the journal_mode PRAGMA.

I'll leave this open until tomorrow to make sure we're getting updated profiles daily.

Comment 53

[Chris Cooper [:coop] (he/him)]

(In reply to Chris Cooper [:coop] from comment #52) 
> I'll leave this open until tomorrow to make sure we're getting updated
> profiles daily.

Looks good.