Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#1527 closed Bug (fixed)

Problems with assets files in installs with a separate webserver

Reported by: ts77 Owned by: JulienM
Priority: normal Milestone: Piwik 1.1
Component: Core Keywords:
Cc: Sensitive: no

Description

For scalability I'm running the webserver on a separate server from the php backends.
That worked well so far as I'm rsyncing every piwik update to the webserver too. No problems with static JS and CSS files.

But the new assets are compiled and built on the php backends as static files which the webserver tries to deliver on his own but he doesn't have the compiled assets yet.
For now I've solved this situation by another rsync which transfers the tmp folder too but I would like some other solution which at least *allows* me to have the assets delivered by php too (even if it only does a passthrough of its compiled assets) so that the php-backends come into play again.

Attachments (1)

proxy.patch (49.9 KB) - added by JulienM 4 years ago.

Download all attachments as: .zip

Change History (30)

comment:1 Changed 4 years ago by matt (mattab)

But the new assets are compiled and built on the php backends as static files which the webserver tries to deliver on his own but he doesn't have the compiled assets yet.

When a server doesn't have the assets, it should generate them. It is important to ensure config files are synched, as assets files depend on which plugins are enabled.

Why are assets not created automatically on all your servers?

comment:2 Changed 4 years ago by ts77

Because the webserver is only delivering static files and dynamic requests are answered from php backends through fastcgi on *other* servers.
The problem is the mixture of static and dynamic files here. Static files are generated from the dynamic requests but static files are not handled by the servers returning the php output.

comment:3 Changed 4 years ago by vipsoft (robocoder)

  • Milestone set to 3 - Piwik 0.9 - Surviving The Wild
  • Owner set to vipsoft

It's doable. Instead of the page linking to the static files, the page could link to .php files that would serve the minified content, e.g., /tmp/assets/{hashcode}.js.php.

(I contemplated something similar, to handle the permissions issue on /tmp/assets.)

comment:4 Changed 4 years ago by matt (mattab)

Is there still a permission issue with assets with the new fixes?

Rather than serving the file from a .php script (we should then ensure that caching works for all etc. which will add complexity), how does the system work to decide which requests go where? is it based on the filename?

comment:5 Changed 4 years ago by vipsoft (robocoder)

The permission problems should now be fixed.

Making the asset cacheable won't be too hard ... I can lift code that
I already wrote in js/index.php.

comment:6 Changed 4 years ago by ts77

Thanks vipsoft for taking this one!

Matt: yes, its filename based but just forwarding the js to the php-backends won't help. They are raw fastcgi php listeners, not full webservers.

comment:7 Changed 4 years ago by matt (mattab)

why don't we call the static CSS file [hash].php ? same for JS? filename extension doesn't matter for browsers, and this would maybe work?

comment:8 Changed 4 years ago by ts77

It will be parsed as php by all webservers then.
Isn't php always sending some headers which avoid caching in the browser and last modified headers and similar won't be sent for dynamic content?
(wouldn't matter for me but just wanted to throw this in)

comment:9 Changed 4 years ago by matt (mattab)

  • Milestone changed from 3 - Piwik 0.9 - Surviving The Wild to 5 - Piwik 1.1

comment:10 Changed 4 years ago by vipsoft (robocoder)

Proposal:

Refactor AssetManager.php into a base class + factory method, and introduce a single config option:

; if blank, disables merging of assets; css and js files are served individually
; merge - js and css assets are minified and merged; by default, content is served directly
; proxy - js and css assets are minified and merged; content is served via a php proxy script
asset_manager = merge

Files:

  • core/AssetManager.php
  • core/AssetManager/Merge.php
  • core/AssetManager/Proxy.php

For the PHP proxy, lift code from js/index.php which sends the Last-Modified header, handles If-Modified-Since (in the request), and Accept-Encoding (on-the-fly compression).

comment:11 Changed 4 years ago by JulienM (JulienMoumne)

Is there really a need to have two mechanisms for handling assets inclusion ?

Wouldn't the php proxy method be sufficient or is there any advantages I don't see in allowing standard includes of merged assets ?

comment:12 Changed 4 years ago by vipsoft (robocoder)

I don't mind if the php proxy serves all use cases. (I wasn't that keen on serving content directly out of tmp.)

Where should the proxy be located? CoreHome? e.g.,

<link rel="stylesheet" type="text/css" href="index.php?module=CoreHome&action=getCss" />
<script type="text/javascript" src="index.php?module=CoreHome&action=getJs"></script>

comment:13 Changed 4 years ago by matt (mattab)

+1 for keeping one mechanism only. Proxy proposal in CoreHome looks good.

comment:14 Changed 4 years ago by JulienM (JulienMoumne)

I implemented the proxy solution proposed by vipsoft.

I had to factorize the code from /js/index.php (cache and compression management) in core/Piwik.php. The content-type is provided as a string, there is always the possibility of using a magic mime database.

comment:15 Changed 4 years ago by JulienM (JulienMoumne)

  • Owner changed from vipsoft to JulienM

comment:16 Changed 4 years ago by ts77

I've applied the patch, thanks a lot for it!
I had one problem though ...
All my systems are running with
zlib.output_compression = On
which leads to garbled (probably double compressed) output of the css and js files.
As a workaround I added
ini_set('zlib.output_compression',0);
here if output compression is enabled in core/Piwik.php.

IMO it would be best if either thats set (dunno if that works everywhere) or additional output compression by piwik is disabled when that variable is detected.

comment:17 Changed 4 years ago by vipsoft (robocoder)

re: zlib.output_compression; yeah, we should test for that; if enabled, let php handle the Accept-Encoding header using its built-in support.

Thanks again Julien.

comment:18 Changed 4 years ago by matt (mattab)

(In [2876]) Gzipped files should be written to /js/ refs #1527

comment:19 Changed 4 years ago by JulienM (JulienMoumne)

Test for zlib.output_compression added.

Compressed files moved to /tmp/.

Vary: Accept-Encoding header added.

gzdeflate used instead of gzcompress because of MSIE misinterpretation of compression RFCs.

comment:20 Changed 4 years ago by vipsoft (robocoder)

Except for some typos (e.g., "satic" and "futur") and excess blank lines, it looks good. Go ahead and check in.

Oh...js/index.php might be missing a closing curly brace. (Hard to tell from the patch.)

Changed 4 years ago by JulienM (JulienMoumne)

comment:21 Changed 4 years ago by ts77

Any chance to include that in 1.0?
Whats the issue why it was moved scheduled to 1.1?

comment:22 Changed 4 years ago by vipsoft (robocoder)

  • Component changed from UI (templates, javascript) to Core

matt and I haven't had time to code review.

you're welcome to apply the patch against 0.9.9 and test it out.

comment:23 Changed 4 years ago by ts77

The patch works fine in my environment, applied against 0.9.9.
I got some rejects but these were just for the file headers.

comment:24 Changed 4 years ago by JulienM (JulienMoumne)

  • Resolution set to fixed
  • Status changed from new to closed

(In [3048]) fixes #1527 - merged assets are now returned to browser using a custom php proxy script, refs #660 - asset manager updated in two ways: - inclusion directive via proxy, - hash management dropped

comment:25 Changed 4 years ago by vipsoft (robocoder)

(In [3054]) refs #1527 - add check for finfo_open, in case PECL extension not installed and not php 5.3.x

comment:26 Changed 4 years ago by vipsoft (robocoder)

(In [3140]) refs #1507, refs #1527 - revert hack from #1507, and fix unit test on Windows per Julien's email

comment:27 Changed 4 years ago by JulienM (JulienMoumne)

(In [3147]) refs #1527 - bypassing a unit test on windows, as far as I know, it is not possible to remove reading rights on a windows file using PHP

comment:28 Changed 3 years ago by vipsoft (robocoder)

(In [3365]) refs #1527 - minor fix detecting when zlib.output_compression is enabled

comment:29 Changed 3 years ago by vipsoft (robocoder)

(In [3976]) refs #1527 - refactoring test for php output compression

Note: See TracTickets for help on using tickets.