Full Page Cache issues #606

getdatakick · 2018-09-25T14:54:02Z

I'm submitting this issue to present the results of my investigation into full page cache

Tasks

Extend cache Key
Remove hook names from section delimiter
On refresh, execute only hooks that are part of cached page
Merchants have no way to tell which hook should be cached or not
Side effects
Switch colors of cached / non-cached hooks
On refresh, instantiate and pass required hook parameters to Hook::exec method

How full page cache works

When full page cache is enabled for some page (controller), the content of the page is stored
in the cache, and is served to all visitors with the same $cacheKey.

We can also choose which display hooks should be considered static, and which are dynamic.
If we make some hook dynamic (green), then the hook output will be wrapped inside html
comments like this:

<html>
...
<!--[hook displayHeader] [id_module 12]-->output of hookDisplay from module 12<!--[hook displayHeader] [id_module 12]-->
<!--[hook displayHeader] [id_module 15]-->output of hookDisplay from module 15<!--[hook displayHeader] [id_module 15-->
...
</html>

When we serve cached version of the page, all dynamic hooks are executed, and up-to-date content is injected into the html page using these comment delimiters.

Cache Key

Cache key consists of these parts:

url - modified page url. Some query parameters can be omitted, if we know they do not participate on page content. For example, urls with different utm_source= can return the same cached version (these parameters are used by google analytics javascript code only)
currency id
language id
shop id
country id

Problems

While this somehow works, there are many problems that should / need to be addressed

Cache key

The first problem is in cache key. For example, it does not contain customer group. So if we have different prices for different groups, we can server wrong content! What else should be part of cache key?

Another small issue is with implementation itself - it's quite brittle, implemented on two places slightly differently. This should be consolidated.

hook name in delimiter

The html comment delimiter contains hook name, and this causes problem when we replace the content with fresh one. The reason is that hooks can have aliases, for example displayHeader have alias Header.

So, in the html content, there can be

<!--[hook displayHeader] [id_module 12]--><!--[hook displayHeader] [id_module 12]-->

but the code that is performing replacement is looking for

<!--[hook Header] [id_module 12]--><!--[hook Header] [id_module 12]-->

This does not match, so thirtybees will return stale/original version of hook output

All green hooks are always executed

This is quite stupid. When we are serving cached version, we go over all green / dynamic hooks, and execute every one of them (even if it was not used in the cached paged).

Not only is this a waste of resources, it can also create various strange bugs. For example, thirtybees can execute hook displayPayment on category page, and this can very
easily result in some 500 error code, because developers expected this hook to be executed only in context of Order controller.

To fix this problem, we should somehow parse the cached page, and retrieve list of hooks to be refreshed. And execute only those.

Merchants have no way to tell which hook should be cached or not

This is a big problem.

Merchants have no clue if the hook content should be cached or not. The only way to tell for sure is to look into the code, and determine if the function is pure or not (if we use functional programming terminology).

What makes this even worse is the fact that all hooks are by default considered pure -- they are all static/red.

To fix this, we should have some additional metadata, so the module developers could tell tb engine how to treat each hook. It should be responsibility of module developers to decide if the hook content should be cached or not.

Obviously, this would work for with native modules only. Legacy / ps16 modules would not be aware of these metadata, and merchant would still need to decide. In this case, it would be best if all hooks would be dynamic/green by default.

Side effects

This is another big problem. Display hooks can have side effects, such as registering new css/javascript files, adding javascript definitions, etc. Example:

public function hookDisplayHeader() {
  if (isLogged) {
    $this->addJS('javascript1.js');
  } else {
    $this->addJS('javascript2.js');
  }
}

Even if we mark this as a dynamic/green hook, it will not work as expected. This hook does not produce any output, it only registers new javascript file. These files are added to html output elsewhere -- but that never happen when we serve cached content.

It's very hard to fix this.

Colors

Maybe it is only me, but I would expect that green means that the hook is cached, and red means hook that is executed on every page request.

The text was updated successfully, but these errors were encountered:

getdatakick · 2018-09-26T06:28:02Z

I just found another issue - and this one is big

(description removed as it's security related --Traumflug)

therampagerado · 2018-09-26T07:51:36Z

Please edit your comment so that one can't find out how to reproduce this issue until it's fixed.

(description removed as it's security related --Traumflug)

Traumflug · 2018-09-26T19:16:25Z

For those who don't follow commits closely, @getdatakick contributed an entire series of commits, which should fix the security issue as well as some of the issues mentioned in the first post here. These commits are:

getdatakick · 2018-09-30T12:37:26Z

Another issue is related to hook parameters. Some hooks expects parameters, for example displayRightColumn expects Cart object, displayShoppingCart expects array with summary information about cart, displayBeforeCarrier requires array of Carrier objects, etc...

When FPC executes non-cached hook, it just calls it without parameters. That, of course, starts chained reaction of errors.

I have no idea how to fix this properly. We can have some helper function for every hook that expects parameters. This function could create a context, and call hooks with the expected parameters. But that would, of course, not work for custom hooks (some modules register their own hooks)

Traumflug · 2018-09-30T19:21:40Z

These hook placeholders in cached HTML could also store hints for required parameters.

Before investing more efforts into these issues I'd love to see some performance measurements. How much faster is such a full cached page actually, especially on front, category and product pages? How much disk space require all the cached pages in all their variants (didn't investigate this yet, but with 10.000 products we might have millions of cache files)? How often do we get cache hits without cache warming? Is cache warming actually possible?

Merchants are pretty keen on this Full Page Cache, so playing with the idea to ditch it means we need solid numbers showing that it's not reliably doable, doesn't give the expected performance improvements and/or there are other opportunities giving equally fast / even faster page loads.

Dh42 · 2018-09-30T21:14:38Z

Its not so much about speed, it is more about the number of users it increases the websites load capacity to. Notice how the full page cache took the number of users from 270 to 563 before the website became unresponsive. https://thirtybees.com/blog/thirty-bees-caches-tested/

It does help with normal page loads as well. I maybe should do a test on that as well.

getdatakick · 2018-10-01T05:19:14Z

These hook placeholders in cached HTML could also store hints for required parameters.

The question is if that's enough. For things like product object or array of product objects, it's probably doable. We know how to create these objects, so we can indeed store some hints.

But what about scalar values? There is no way (in general) to recreate them. We can, of course, store those scalar value directly inside template. But is that correct? It will probably work in like 95% of the time, as those scalar values are probably derived from the rendered entity, but not always.

As an example, imagine you want to display random product on your home page. You will use some hook to render this product, and this hook takes product id as parameter. To do this, one would put something like this into homepage/index template:

{hook h='displayProductSummary' productId=$randomProductId}

Now, if this hook is cached/green, the home page will obviously always display the same product. Not random. If we make it non-cached, then we would store $randomProductId value inside rendered template, and then on every page render we would call displayProductSummary with this value. Which again will always display same product.

I know this is weird scenario and that nobody would probably implement this functionality this way. Anyway, I think it demonstrates the problem with this approach nicely.

There's really not much we can do about this, because displayProductSummary is not core hook and we know absolutely nothing about it, or its parameters.

Traumflug · 2018-10-01T11:20:42Z

Notice how the full page cache took the number of users from 270 to 563 before the website became unresponsive.

So I took my default installation, turned off debugging, turned on profiling and tried cache settings. These screenshots taken for loading product page "Honey", reloaded three times with some 5 seconds.

Server side cache off, full page cache off:

$ for F in {1..5}; do time curl http://localhost/thirtybees/en/coffee-and-tea/tea/honey >/dev/null 2>&1; done 2>&1 | grep real
real	0m0,236s
real	0m0,219s
real	0m0,213s
real	0m0,198s
real	0m0,200s

Server side cache on (filesystem, depth 1), full page cache off:

real	0m0,260s
real	0m0,233s
real	0m0,201s
real	0m0,216s
real	0m0,197s

Server side cache on, full page cache on, all hooks red:

real	0m0,259s
real	0m0,202s
real	0m0,176s
real	0m0,178s
real	0m0,181s

Server side cache on, full page cache on, all hooks green:

real	0m0,243s
real	0m0,092s
real	0m0,094s
real	0m0,132s
real	0m0,097s

As one can see, FPC gives no advantage on the first load and just 10% with hooks served on subsequent loads. Only skipping all the hooks gives a significant advantage.

Before measuring on the command line I tried with the built in profiling tool, but this apparently doesn't work with FPC on.

getdatakick · 2018-10-01T12:18:58Z

These numbers are what would one expect, for a test in isolation. The question is how this behave under heavy load. I'd assume the numbers will be quite different, since FPC will bypass the DB.

@Dh42 you have some experience in this kind of testing, right?

Dh42 · 2018-10-01T12:57:02Z

I do, that link I posted, https://thirtybees.com/blog/thirty-bees-caches-tested/ I actually tested a 2gb vultr vps using locust.io

I didn't test the initial loading, I just tested the ability for handling high loads. Since the database is by passed it really effectively doubles what the server can handle before falling offline.

Traumflug · 2018-10-01T12:58:03Z

One can do this with 20 parallel requests as well, with about equivalent results.

Script:
time_requests.txt

Server side cache off, full page cache off:

for F in {1..5}; do time ./time_requests.txt; done 2>&1 | grep '^real'
real	0m2,600s
real	0m2,819s
real	0m2,770s
real	0m2,604s
real	0m2,529s

Server side cache on (filesystem, depth 1), full page cache off:

real	0m2,547s
real	0m3,170s
real	0m2,754s
real	0m2,644s
real	0m2,591s

Server side cache on, full page cache on, all hooks red:

real	0m2,716s
real	0m2,658s
real	0m2,607s
real	0m2,203s
real	0m2,220s

Server side cache on, full page cache on, all hooks green:

real	0m1,066s
real	0m1,072s
real	0m1,090s
real	0m1,159s
real	0m1,190s

Traumflug · 2018-10-01T13:03:59Z

Since the database is by passed it really effectively doubles what the server can handle before falling offline.

Taking my simple measurements into account, it looks more like the performance improvement isn't a result of bypassing the database, but of bypassing all the hooks. At the time of your measurements, hooks were off by default (red). Now green means off.

Dh42 · 2018-10-01T13:12:38Z

That could be the case, but I did a full test, hitting pages that would make database requests, I did not just test the home page in the tests, I tested the home page, a search page, the category page, and the product page. So at least on the search page database requests were made.

chasevasic · 2018-10-25T19:02:56Z

Can I get an update on this bug from @getdatakick and/or @Traumflug ? I'm a little bit unclear on if full page caching is usable now. I can volunteer some testing if it would help.

The 1.0.7 release notes mention that full page cache is working, however, this bug is still open. From the conversation here it seems that only the security issue was fixed, while more problems remain.

As far as the importance of full page caching goes, I will show some results from tests run on a site I'm working with:
All numbers are averages, local caching disabled, remote server with debugging settings turned on. For these tests, all hooks were the default red (on version 1.0.7, so red = on?). The only caching used for test purposes was FPC with Redis

A particularly bad product page
FPC Off - 10374ms
FPC On - 460ms

A category page
FPC Off - 4656ms
FPC On - 144ms

Front page
FPC Off - 999ms
FPC ON - 287ms

Preliminary testing for us shows massive results. Again, I can do more tests if it's helpful. It's worth mentioning that these tests were done on a development site with little optimization though, and FPC has more of an effect since it's the only caching or optimization in place.

therampagerado · 2018-10-25T19:13:12Z

It's still broken, it gives 500 on some pages. It's not stable and it's not recommended to use it.

Purity1 · 2018-10-25T19:41:51Z

As therampagerado said, it gives 500 error in some situations.
Not usable.

getdatakick · 2019-01-23T11:40:14Z

The problems with parameter passing should be fixed by aaa1730

getdatakick mentioned this issue Sep 25, 2018

Full page cache prohibits Captchas #594

Open

Traumflug added the Enhancement label Aug 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Full Page Cache issues #606

Full Page Cache issues #606

getdatakick commented Sep 25, 2018 •

edited

getdatakick commented Sep 26, 2018 •

edited by Traumflug

therampagerado commented Sep 26, 2018 •

edited by Traumflug

Traumflug commented Sep 26, 2018

getdatakick commented Sep 30, 2018

Traumflug commented Sep 30, 2018

Dh42 commented Sep 30, 2018

getdatakick commented Oct 1, 2018

Traumflug commented Oct 1, 2018

getdatakick commented Oct 1, 2018

Dh42 commented Oct 1, 2018

Traumflug commented Oct 1, 2018

Traumflug commented Oct 1, 2018

Dh42 commented Oct 1, 2018

chasevasic commented Oct 25, 2018 •

edited

therampagerado commented Oct 25, 2018

Purity1 commented Oct 25, 2018

getdatakick commented Jan 23, 2019

Full Page Cache issues #606

Full Page Cache issues #606

Comments

getdatakick commented Sep 25, 2018 • edited

Tasks

How full page cache works

Cache Key

Problems

Cache key

hook name in delimiter

All green hooks are always executed

Merchants have no way to tell which hook should be cached or not

Side effects

Colors

getdatakick commented Sep 26, 2018 • edited by Traumflug

therampagerado commented Sep 26, 2018 • edited by Traumflug

Traumflug commented Sep 26, 2018

getdatakick commented Sep 30, 2018

Traumflug commented Sep 30, 2018

Dh42 commented Sep 30, 2018

getdatakick commented Oct 1, 2018

Traumflug commented Oct 1, 2018

getdatakick commented Oct 1, 2018

Dh42 commented Oct 1, 2018

Traumflug commented Oct 1, 2018

Traumflug commented Oct 1, 2018

Dh42 commented Oct 1, 2018

chasevasic commented Oct 25, 2018 • edited

therampagerado commented Oct 25, 2018

Purity1 commented Oct 25, 2018

getdatakick commented Jan 23, 2019

getdatakick commented Sep 25, 2018 •

edited

getdatakick commented Sep 26, 2018 •

edited by Traumflug

therampagerado commented Sep 26, 2018 •

edited by Traumflug

chasevasic commented Oct 25, 2018 •

edited