Re: [PATCH] [RFC] custom error pages

On 11/21/2010 05:21 PM, Amos Jeffries wrote:
> On Sun, 21 Nov 2010 14:50:20 +0100, Henrik Nordström
> <henrik_at_henriknordstrom.net> wrote:
>> lör 2010-11-20 klockan 18:58 +1300 skrev Amos Jeffries:
>>
>>> To simplify the error page loading a symlink is added from the
> templated
>>> /usr/share/squid/errors/local to that /etc/squid/errors.d folder and
>>> checked just before loading the errors/templates/ backup.
>>
>> Why? I find having this symlink confusing, and yet another potential
>> source of configuration errors.
>
> Okay. As Robert pointed out the symlink is not going to work everywhere.
> If the idea is agreed I'll go back to an older idea of a separate full
> lookup path and some further performance optimisations.
>
>
> As to why. We currently have admin who still:
> a) want to replace the entire template set with something else
> b) want to replace just some of the template set with something else
> c) want to add local deny_info ERR_* pages with alternative names
>
> The error_directory + auto-negotiate currently caters for (a) and (c) but
> not (b).

For (b), the admin who wants to replace some errors, can already do that
from the Squid startup script:

    cp -r standard_errors/* error_directory/
    cp -r my_errors/* error_directory/
    start squid

Also, the error_directory option in squid.conf allows more flexibility
for multi-instance and SMP Squids than the proposed hard-coded directory
name. I think there are cases where different Squid instances can
benefit from different error pages.

Are you sure replacing error_directory with a hard-coded location is a
step forward?

> This replaces error_directory with a fixed location which can be used to
> override auto-negotiate for cases (a) and (b). And also as a known location
> for case (c).
>
> My earlier idea was to let them add to errors/templates/ but some distros
> need to remove or replace that folder when upgrading the langpack content.
>
> Placing this under /etc/squid/errors.d keeps everything which is manually
> configurable in one area. So the admin no longer even really needs to worry
> about /usr/shared/squid/errors/ or what is in there.

A custom script like the sketch shown above would accomplish the same,
right?

> The patch here is missing a few possible performance optimizations.
> The first I intend to get in with a re-submission if this idea is
> accepted. That is to add a flag to the template data loaded on
> initialization to indicate whether its used unconditionally (override) or
> auto-negotiate is to happen first (presently only loaded as the final
> templates/* backup). This removes 1-2 cycles of file lookup on each error
> request.
> Second was suggested by Adrian way back. To load the entire template set
> into a memory tree and search that. This is a bit more work and requires
> some performance/cost benefit analysis I don't have time for.
> Third is a smaller alternative to the second, just to cache the languages
> which are available. Thus reducing some useless file probes. We have most
> of the major web languages and their variants, but there are a few which
> still hit two file loads.

Opening files for each runtime error is pretty bad from performance
point of view, although we can hope that errors do not happen often
enough to cause problems in most setups. Preloading is a better
approach, IMO, but is outside this patch scope.

AFAICT, your patch will add to that runtime file opening overhead, even
if the user does not have any custom pages, because Squid will need to
check whether there is a custom file in the new directory. Not a big
deal if we find file opening overhead acceptable in general, but perhaps
it should be documented in the commit message.

Thank you,

Alex.
Received on Thu Nov 25 2010 - 22:52:10 MST