Re: [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when regenerating cache (WIP)

From: Jakub Narebski <jnareb@gmail.com>
Date: Jan 26, 2010, 02:30 UTC
Message-ID: <201001260330.26365.jnareb@gmail.com>
In-Reply-To: <4B5E01F5.20402@eaglescrag.net>

On Mon, 25 Jan 2010, J.H. wrote:

Show 45 quoted lines

> On 01/25/2010 05:48 AM, Jakub Narebski wrote:
>> On Mon, Jan 25, 2010, Petr Baudis wrote:
>>> On Mon, Jan 25, 2010 at 12:46:39PM +0100, Jakub Narebski wrote:
>>>> On Sun, 24 Jan 2010, Petr Baudis wrote:
>> 
>>>>>   I have stupid question, common to both the original patch and this
>>>>> RFC.
>>>>>
>>>>>> [RFC PATCH 10/10] gitweb: Show appropriate "Generating..." page when
>>>>>> regenerating cache (WIP)
>>>>>
>>>>>   Just why is a "Generating..." page appropriate?
>>>>>
>>>>>   I have to admit I hate it; can you please at least make it
>>>>> configurable? Why is it needed at all? It [...] confuses
>>>>> non-interactive HTTP clients [...]
>> 
>>>> Second, gitweb can always check User-Agent header, and serve 
>>>> "Generating..." page only to web browsers:
>>>>
>>>>   unless (defined $cgi->user_agent() &&
>>>>           $cgi->user_agent() =~ /\b(Mozilla|Opera)\b/i) {
>>>>   	return;
>>>>   }
>>>>
>>>> or something like that.
>>>
>>> I'm not too happy with this. What about Safari? Opera? ELinks? There's a
>>> lot of web browsers.
>> 
>> The "Mozilla" part would catch all "Mozilla compatibile" web browsers,
>> including Firefox (and other Gecko-based web browsers), Internet Explorer,
>> WebKit based browsers including Safari and Chrome and Konqueror.
>> The "Opera" part would catch Opera.
>> http://www.nczonline.net/blog/2010/01/12/history-of-the-user-agent-string/
>> 
>> As to other web browsers like Elinks, Lynx, w3m, Dillo, etc.: the issue
>> is whether they honor '<meta http-equiv="refresh" content="0" />'.  
>> I think it is better to stay on the safe side; it is not disaster if web
>> browser is not shown "Generating..." page where it could (but see 
>> explanation below).
> 
> Most of them do, that particular tag has been around for a long time and
> since it doesn't require Javascript to do the page refresh it's pretty
> much universal.

For this method to work there is additional assumption about web browser, namely that it would follow refresh only after it gets page in full (only when connection closes). But perhaps that is true for all web browsers following http-equiv refresh.

Show 8 quoted lines

> 
> The problem is going to be with things like wget when someone wants to
> snag a binary file.  This works fine if the file is already cached, but
> the user doesn't get what they are expecting if they get a blob that
> isn't the final file, but the html contents of the page.  I don't know
> of any hint that things like wget would send to the server that you
> could switch based on, but it would be more or less the non-background
> caching state.

Well, the above solution would work as long as you don't run wget with --user-agent.

[...]

Show 11 quoted lines

>>> But if it will be possible to turn this off eventually, it's all your
>>> call whether to bother implementing it. :-)
>> 
>> In my implementation it is (or rather would be) as simple as just
>> not passing 'generating_info' => \&git_generating_data_html in the
>> GitwebCache::SimpleFileCache constructor.
> 
> At least in mine it was don't allow background caching.  It would force
> everyone to wait on the one process that was actually generating
> content.  But it means a few blank pages with a spinning working icon
> until the cache releases it's exclusive lock.

Theoretically at least having "Generating..." page is orthogonal to using locking, to background serving and to serving stale data. In current implementation those features are dependent, serving stale data on locking, and "Generating..." also on locking.

I think that serving stale data should depend on background caching, as otherwise we introduce inequality in treating process which regenerates cache (and must wait without background caching) and rest of processes (which are served stale data).

Certainly in final version of patch all this should be made configurable.

 
P.S. With background caching there is a question whether parent should
generate and child display stale or "Generating...", or whether parent
should display stale/activity and child (re)generate cache entry.  Also
there is an issue whether parent should waitpid for child, would fiddling
with SIGCHLD handler be enough, or is it not needed.

-- 
Jakub Narebski
Poland