Page MenuHomePhabricator

Work to reduce smarty cruft on the server
Closed, ResolvedPublic

Description

We had a big server snag when clearing caches for the first time post BE. There are a few upstream discussions / patches / techniques I can dig into to try to improve this

Details

Related Gerrit Patches:
wikimedia/fundraising/crm/civicrm : masterDon't parse non-smarty through smarty

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 1 2020, 10:44 PM
Ejegg added a subscriber: Ejegg.Jan 1 2020, 10:46 PM

Is part of the problem that Civi is rendering templates when they will likely never be shown to a user, i.e. during background deduplication jobs?

So this is one of the patches I was thinking we could dig into https://github.com/civicrm/civicrm-packages/pull/234

This is the other https://github.com/civicrm/civicrm-core/pull/14706

It's also possible we are processing greeting templates during dedupe - I think the first of these would fix that

(there are 2 other open PRs against smarty - might be good for this to dig into those too)

Jgreen added a subscriber: Jgreen.Jan 2 2020, 12:21 AM

Chattering in IRC, it sounds as though we should set up an interim fix--a process-control task to purge cruft either by drush or by a bash find & delete. I'm happy to help set that up if I can get some guidance on what to purge and when.

.Nafees791 renamed this task from Work to reduce smarty cruft on the server to ruri.Jan 23 2020, 4:36 AM
.Nafees791 closed this task as Declined.
.Nafees791 triaged this task as High priority.
.Nafees791 updated the task description. (Show Details)
ArielGlenn renamed this task from ruri to Work to reduce smarty cruft on the server .Jan 23 2020, 9:26 AM
ArielGlenn reopened this task as Open.
ArielGlenn raised the priority of this task from High to Needs Triage.
ArielGlenn updated the task description. (Show Details)
Aklapper renamed this task from Work to reduce smarty cruft on the server to Work to reduce smarty cruft on the server.Jan 23 2020, 10:19 AM
Eileenmcnaughton added a comment.EditedTue, Mar 10, 4:05 AM

OK so plan is to add

define( 'CIVICRM_TEMPLATE_COMPILE_CHECK', FALSE);

to our CiviCRM settings.php file

The Smarty documentation recommends that for production per - https://www.smarty.net/docs/en/variable.compile.check.tpl & my digging has not come up with any counter-indications.

https://github.com/civicrm/civicrm-core/pull/14706

Separately I have a PR to reduce smarty processing

It would be nice to see if there is a performance impact

Ejegg added a comment.Tue, Mar 10, 3:47 PM

Weird, that COMPILE_CHECK constant looks like it would INCREASE disk access - it checks the template file timestamp on every usage including when it would otherwise just use the cached version.

Ejegg added a comment.Tue, Mar 10, 3:48 PM

Ahh, and that documentation page suggests setting it to FALSE in production:

Be sure to set $compile_check to FALSE for maximum performance.

Change 578626 had a related patch set uploaded (by Eileen; owner: Eileen):
[wikimedia/fundraising/crm/civicrm@master] Don't parse non-smarty through smarty

https://gerrit.wikimedia.org/r/578626

Change 578626 merged by jenkins-bot:
[wikimedia/fundraising/crm/civicrm@master] Don't parse non-smarty through smarty

https://gerrit.wikimedia.org/r/578626

@Jgreen or @Dwisehaupt can I get one of you to delete the folders on live under

sites/default/files/civicrm/templates_c/en_US

(well everything under that directory).

Smarty creates files when it caches stuff. When I clear caches it deletes the files. However, it doesn't delete the folders it seems. I've deployed changes to dramatically reduce how many files it creates so clearing out the old folders will make it easier to see what is being re-created post change

The files I am seeing in there now are

templates_c/en_US$ ls -R * | grep php
029CEF80%%footer.tpl.php
0693F89E%%drupal.tpl.php
0CBEC124%%default.tpl.php
31894DFC%%UpdateConfigBackend.tpl.php
4DC76B26%%body.tpl.php
529A2042%%l10n.js.tpl.php
61124474%%notifications.tpl.php
63D062BA%%LangSwitch.tpl.php
79B5A8D2%%Dashboard.tpl.php
8B6A05F3%%CreateNew.tpl.php
938D45F8%%Subject.tpl.php
9D3B6E05%%RecentlyViewed.tpl.php
B75FFEFA%%Add.tpl.php
C6B24099%%status.tpl.php
E8D2BCB1%%accesskeys.tpl.php
ED78F5CC%%formButtons.tpl.php
F77C7890%%CMSPrint.tpl.php
FD2F2A8F%%info.tpl.php

This is a BIG improvement

@Eileenmcnaughton Just to be clear, this is on the civi host, not the qa host, yes?

yes - the templates_c directory has just smarty cache templates in it

Cool. Empty dirs have been cleaned up and I'm not seeing any empty dirs currently.

I still have a bit of upstreaming discussion going on but this is no longer over-using Smarty. We should see some performance improvement on queues and especially dedupinng

I'm going to close this - I don't think we can keep our tasks open pending upstreaming

I just rechecked & only useful smarty stuff is recreated

Eileenmcnaughton closed this task as Resolved.Wed, Mar 11, 9:41 PM