887117 - Move the compiled template cache dir to a directory in the document root to avoid storing on NFS

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, defect)

Description

[David Lawrence [:dkl]]

To alleviate the possibility of NFS causing issues when the webheads try to access the compiled templates stored in data (which is shared by all webheads), each webhead should store their cached templates locally. The compiled templates are not difficult to regenerate and if code pushes happen properly, they should all be the same without the need of network file share. We can create a small utility script that will just do the template compilation step without the need to run checksetup.pl on all webheads separately. 

We will need to file a bug with webops to also update their code push tools to run the template compilation script as well as fixperms.pl after the new code is in place on each webhead.

Patch coming.

dkl

Comment 1

[David Lawrence [:dkl]]

Attachment: Patch to move compiled templates to template_cache dir (v1)

Once this looks good, I will file a bug with IT to update their push scripts accordingly.

dkl

Comment 2

[David Lawrence [:dkl]]

Glob mentioned another alternative location for these templates could be /tmp which could be doable as well. Bonus points for using tmpfs. Jake, do you have a preference for these type of things? How do the other web properties who cache their templates in a similar fashion handle the location for the  best performance?

dkl

Comment 3

[Jake Maul [:jakem]]

It kinda depends on how the cache files are generated.

The most common solution is that the admin node generates them. In this case, they just live somewhere within the site (/data/www/<site>/cache or similar), because that's easiest to deploy... it takes zero extra work to send those files out, they go out just like any other file in the site.

If the files are purely generated by the web code on-demand, that will still work, but sometimes requires a little more care... some of our sites deploy with rsync, which will wipe out the cache in this case. Bugzilla deploys via an internal git repo, so this isn't a significant concern and this is still feasible. When it is a concern, we typically go with a directory in /tmp.

We don't usually use tmpfs- /tmp isn't set up that way by default, and using it forces you to deal with each possible thing that uses it... across a whole infra, that doesn't scale well. Instead, we just let /tmp be disk, and let the normal Linux VFS cache deal with whether or not a particular file needs to be in memory. If the files are frequently accessed, it'll cache them. :)


My real question is, do we still want to bother with this, given this morning's revelation regarding load balancer bandwidth?

Comment 4

[David Lawrence [:dkl]]

(In reply to Jake Maul [:jakem] from comment #3)
> My real question is, do we still want to bother with this, given this
> morning's revelation regarding load balancer bandwidth?

Well practical thinkers would say, do not do this now as it is not broken. But I still would like to think that this would give some improvement over loading
these templates over a network link even though people state NFS is not that slow.

I do not load ./data over a network connection locally so moving ./data/template to ./template_cache (or even /tmp/template_cache) will make zero difference to me. But on a high traffic cluster such as BMO it could possibly be noticable. Also the chance of other systems messing with permissions, etc. become zero as well. 

We could switch to this new configuration for a bit, see if it makes any difference even the slightest. Ashish says it is not a big deal to add the new script to precompile the template cache on each code push so we can remove that if we ever want to switch back to the current way.

Any objections?

dkl

Comment 5

[:glob ✱]

Comment on attachment 767596 [details] [diff] [review]
Patch to move compiled templates to template_cache dir (v1)

i think at this point the gains are not worth the wins for this change.

Comment 6

[David Lawrence [:dkl]]

Sounds fair.

Comment 7

[:glob ✱]

i'd like to revisit this fix, as i'm seeing race-conditions with templates being recreated.

Comment 8

[:glob ✱]

Comment on attachment 767596 [details] [diff] [review]
Patch to move compiled templates to template_cache dir (v1)

r- stands, as the main .htaccess needs to be updated to deny all access to template-cache/

Comment 9

[Dave Miller [:justdave]]

First I've seen this bug because I was behind on bugmail....  We used to do this and it got lost when we moved out of phx1, apparently.

The previous solution had the template cache at /data_local/template (or something similar to that) and data/template within the docroot was bind-mounted by rc.local (to make sure the NFS mount for data was in place first before the bind mount happened).  The push scripts would nuke the template cache immediately after restarting apache (IIRC).

Comment 10

[Shyam Mani [:fox2mike]]

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #9)
> First I've seen this bug because I was behind on bugmail....  We used to do
> this and it got lost when we moved out of phx1, apparently.

Add this to the list of "awesome things justdave does, but never documents" :p

Comment 11

[David Lawrence [:dkl]]

(In reply to Dave Miller [:justdave] ([e-mail]) from comment #9)
> First I've seen this bug because I was behind on bugmail....  We used to do
> this and it got lost when we moved out of phx1, apparently.
> 
> The previous solution had the template cache at /data_local/template (or
> something similar to that) and data/template within the docroot was
> bind-mounted by rc.local (to make sure the NFS mount for data was in place
> first before the bind mount happened).  The push scripts would nuke the
> template cache immediately after restarting apache (IIRC).

Glob and I discussed this a little and feel it is better to just have a separate dir out of /data and not worry with OS level specific configurations so that it remains generic. It is not that difficult to do it code wise. Admittedly for consistency it is better to have it in data as that is where most dynamic data is stored for a Bugzilla site but 
we for BMO that isn't possible currently. 

dkl

Comment 12

[David Lawrence [:dkl]]

Attachment: 887117_2.patch

(In reply to Byron Jones ‹:glob› from comment #8)
> r- stands, as the main .htaccess needs to be updated to deny all access to
> template-cache/

I was not able to figure out how to block access to a subdirectory from the main .htaccess file as the <Directory> directive will not work in an .htaccess file as you are essentially already in a <Directory> parent block.

Also simply having Bugzilla::Install::Filesystem::create_htaccess create a .htaccess file in the template_cache dir would not work as precompile_templates blows away the directory before recompiling so I had to add a hack to call create_htaccess again.

Maybe you can see a more elegant solution that I am missing without having to refactor quite a bit more code.

dkl

Comment 13

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #12)
> Created attachment 810377 [details] [diff] [review]
> I was not able to figure out how to block access to a subdirectory from the
> main .htaccess file as the <Directory> directive will not work in an
> .htaccess file as you are essentially already in a <Directory> parent block.

RewriteRule ^template-cache/ - [F,L,NC]

Comment 14

[:glob ✱]

Comment on attachment 810377 [details] [diff] [review]
887117_2.patch

as discussed, let's go with the rewriterule to minimise the changes.

Comment 15

[David Lawrence [:dkl]]

Attachment: 887117_3.patch

Comment 16

[:glob ✱]

Comment on attachment 811139 [details] [diff] [review]
887117_3.patch

the permissions of the template_cache directory need to be fixed to allow the web server to write to it.

Comment 17

[David Lawrence [:dkl]]

Attachment: 887117_4.patch

Comment 18

[:glob ✱]

Comment on attachment 818088 [details] [diff] [review]
887117_4.patch

r=glob

Comment 19

[David Lawrence [:dkl]]

Committing to: bzr+ssh://[e-mail]/bmo/4.2
modified .htaccess
modified Bugzilla/Constants.pm
modified Bugzilla/Install/Filesystem.pm
Committed revision 9079.

Comment 20

[David Lawrence [:dkl]]

Reopened as this caused a permission error on bugzilla.allizom.org when it update automatically:

drwxr-xr-x 5 root root 4096 Oct 17 08:30 template_cache/

They should be 

drwxrwx--- 5 root apache 4096 Oct 17 08:30 template_cache/

Will get with IT on this before the next push hopefully.

dkl

Comment 21

[:glob ✱]

looks like what we need to do is compile the templates on each workstation.

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
modified .bzrignore
modified .htaccess
modified Bugzilla/Constants.pm
added contrib/compile-templates.pl
Committed revision 9118.


ashish, can you create /data/bin/sync-extras.sh which cd's into the bugzilla directory and runs:
  ./contrib/compile-templates.pl

it's also a good idea to configure either rsync or git to ignore the template_cache directory, as there's no need for that to be pushed out to the webheads once sync-extras.sh is in place.

Comment 22

[:glob ✱]

after chatting with dkl about this, we realised it would be easier to not pre-compile the templates, instead let the compile on demand.

Committing to: bzr+ssh://[e-mail]/bmo/4.2/
renamed contrib/compile-templates.pl => contrib/clear-templates.pl
Committed revision 9119.

i've renamed the script to "clear-templates.pl".

it'll still need to be executed by sync-extras.sh :)

Comment 23

[:glob ✱]

i've filed bug 934521 for the webops work, marking as r/f.

Comment 24

[Ashish Vijayaram [:ashish]]

Stale needinfo?, my bad. However requested changes were implemented and verified in BMO's push scripts.