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

From: Jakub Narebski <jnareb@gmail.com>
Date: Jan 26, 2010, 01:49 UTC
Message-ID: <201001260249.36678.jnareb@gmail.com>
In-Reply-To: <4B5DFFE5.6060908@eaglescrag.net>

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

> On 01/25/2010 05:56 AM, Petr Baudis wrote:
>> On Mon, Jan 25, 2010 at 02:48:26PM +0100, Jakub Narebski wrote:

Show 28 quoted lines

>>> Now those patches (mine and J.H. both) make gitweb use locking
>>> (it is IIRC configurable in J.H. patch) to make only one process
>>> generate the page if it is missing from cache, or is stale.  Now
>>> if it is missing, we have to wait until it is generated in full
>>> before being able to show it to client.  While it is possible to
>>> "tee" output (using PerlIO::tee, or Capture::Tiny, or tie like
>>> CGI::Cache) writing it simultaneously to browser and to cache for 
>>> the process that is generating data, it is as far as I understand
>>> it impossible for processes which are waiting for data.  Therefore
>>> the need for "Generating..." page, so the user does not think that
>>> web server hung or something, and is not generating output.
>> 
>> Ah, ok, so the message is there to cover up for a technical problem. ;-)
>> I didn't quite realize. Then, it would be great to tweak the mechanisms
>> so that the user does not really have to wait.
> 
> No, that is an incorrect assumption on how the 'Generating...' page
> works, and your missing a bit of the point.
> 
> (1) The message itself 'Generating...' is a cue to the user that
> something is happening and that the browser is not actually hanging.
> Web users are at the point where if things are not instantaneous and
> show immediately they will either browse away completely or hit the
> refresh button incessantly until content does appear.  While the page is
> usually only seen for about a second, and I'll admit it can be annoying,
> it's nothing more than a 'sit tight a second'.  For things like the
> front page it can take upwards of 7 seconds to generate for a single
> user, a lot to ask for a no response scenario.

Pasky is asking later whether instead of separate "Generating..." page (be it server side using trick with http-equiv refresh taking place when page/connection is closed, or JavaScript Ajax-y one) couldn't gitweb use streaming and have early parts of generated page (in particular the page header and navigation bar) serve as activity indicator. I think it is possible, if a bit more complicated, and such feature can be introduced in a separate commit.

As to 'sit tight for a second': the code can be modified to print/show "Generating..." page only if we are waiting more than second (or more than half of second with utime from Time::HiRes) for the data. Pasky, would such change address some of your concerns about "Generating..." page as activity indicator?

Show 11 quoted lines

> 
> (2) It prevents the stampeding herd problem, which was very vehemently
> discussed 4 years ago by HPA and myself and roughly boils down to this:
> 
> When a single user comes into the site, in particular the front page, it
> kicks off a process that will start to generate at it, causing a huge
> amount of git requests into individual repositories and a lot of disk
> i/o.  A second user will then come in and the same requests will start
> to be done from the beginning again, and so on until you basically kill
> the machine because the disk i/o goes up enough that it can't ever
> service the requests fast enough.

As you can see from the way my proof of concept (of splitting) caching series is split, I think that those two features are separate. Using locking to have one writer, and protect against cache miss stampede problem is one issue, and having "Generating..." page is another issue. They are not orthogonal as they are now, as "Generating..." page depends on locking to check if page got generated, but it could simply check whether the cache file is present already.

Show 8 quoted lines

> 
> This does 2 things in the end:
> 
> 1) means there's only 1 copy of the page ever being generated, thus
> meaning there isn't extraneous and dangerous disk i/o going on on the system
> 
> 2) prevents a user from reporting to the website that it's broken by
> giving them a visual cue that things aren't broken.

1) is about locking ('expires_variance' can help there, too), 2) is about
"Generating..." but could be done with streaming: "tee"/"tail" solution.

Show 21 quoted lines

>> So, I wonder about two things:
>> 
>> (i) How often does it happen that two requests for the same page are
>> received? Has anyone measured it? Or is at least able to make
>> a minimally educated guess? IOW, isn't this premature optimization?
> 
> For most pages, not many but it happens more often than you think.  The
> data I have is much too old to be useful now but the front page could,
> at times, have up to 30 people waiting for it without caching.  This is
> a very important patch believe it or not.  For a site the size of
> kernel.org it cannot exist without this.
> 
> But here's a quick stat, in 36 hours git.kernel.org has had
> 156099 accesses world wide or about 1.2 accesses a second.
> 
> android.git.kernel.org, in the same time period has had 115818 accesses.
> 
> If the first request takes 7 seconds to generate, by the time it's done
> there are now 3 additional requests running.  If it again takes 7
> seconds to generate there are now another 3 requests running, etc.  Very
> quickly you've got so much i/o running the box more or less is useless.

On the other hand do cache for front page (projects list) ever expire so that there are no stale data to serve? So the issue with growing I/O is I guess only with priming the cache.

BTW. I wonder how well the 'expires_variance' solution used in CHI would protect against cache miss stampedes; that probably depends on the ratio of variance to time it takes gitweb to generate the page.

Show 5 quoted lines

>> (ii) Can't the locked gitwebs do the equivalent of tail -f?
> 
> Not really going to help much, most of the gitweb operations won't
> output much of anything beyond the header until it's collected all of
> the data it needs anyway and then there will be a flurry of output.

Well, it might be enough to have headers as activity indicator... but on the other hand users can think that server is broken if they see no further data, as opposed to the "Generating..." page.

> It also means that this 'Generating...' page will only work for caching
> schemes that tail can read out of, which I'm not sure it would work all
> that well with things like memcached or a non-custom caching layer where
> we don't necessarily have direct access to the file being written to.

I don't think that caching interfaces such as CHI or Cache::Cache, or individual caching engines such as Cache::Memcached or Cache::FastMmap offer streaming access. OTOH we can read from the side as the data is generated ("tee" to a temporary file, or something), but it is even more complication.

Show 8 quoted lines

> 
> At least the way I had it (and I'll admit I haven't read through Jakub's
> re-working of my patches so I don't know if it's still there) is that
> with background caching you only get the 'Generating...' page if it's
> new or the content is grossly out of data.  If it's a popular page and
> it's not grossly out of date it shows you the 'stale' data while it
> generates the new content in the background anyway, only locking you out
> when the new file is being written.  Or at least that's how I had it.

It is there, i.e. gitweb would serve stale data if it can be found, but it lacks 'too stale to serve as stale data' timeout/expire time. I skipped this feature to get split series out of door faster.

-- 
Jakub Narebski
Poland