Re: [PATCH] Fix session helper "crashing too rapidly"

On Mon, 2011-09-19 at 10:49 +1200, Amos Jeffries wrote:
> The session helper in Squid-3 is concurrent.

Ah, okay.

> The user_key is the opaque
> channel-ID. (Probably should be renamed to match the protocol
> documentation).
> http://wiki.squid-cache.org/Features/AddonHelpers#Access_Control_.28ACL.29

I've renamed to channel_id

> The correct way to fix this is to detect the segfault case add a stderr
> ERROR: message that the helper is concurrent and requires a config
> update.

I've added a fatal error message to STDERR, but I can't get it to output
anywhere (see below).

> stderr should appear in cache.log whenever sent.

Hmmm, it doesn't seem to be.

> Most of the lines so
> far appear to be debug messages

Does that include the failure to open a database? This is written to
STDERR as per any other message, but it does not make it anywhere.

> , which depend on the -d option to
> display.

Do you mean the -d option to the Squid binary? If so, this doesn't seem
to make any difference; it just prints all the log messages to the
display as well as the log file.

I've tried increasing the log level to 9, but to no avail.

> Also....the .8 manual needs to mention the concurrency rather than just
> implying it in the example config.

Done. Also, the following page should be updated:

http://wiki.squid-cache.org/ConfigExamples/Portal/Splash

I'm happy to do it myself, if you can give me wiki edit rights?

> The helper version should get a bump
> to 1.1 as well.

Done in the manual. Is it specified anywhere else as well?

I've also added:

- A new option: "-h". This causes a "hard" timeout, regardless of user
activity. This is because, for many uses of a splash page (adverts etc)
one would want the message to displayed every n hours, regardless of
user activity (certainly that is my requirement).

- A db->sync command. I have found that with more than one child
started, that they do not necessarily share data because each child's
data has not been flushed to disk. This fixes that. However, I am not
sure whether it has other implications, such as much greater disk
overhead. Do you think it should be a configurable option?

Is there anything else that needs updating? RELEASENOTES.html?

Please find attached updated patch for comment.

Thanks,

Andy