Rather major bug in template loading in Desert / Rails 2.2...

[Simon Russell]

Hi there,

After having major problems in production where we weren't able to
respond to more than a few requests a second; I did some profiling.

Nothing in our code seemed to be the cause. After putting my own code
into the Rails Dispatcher, wrapping the call to the controller in
ruby_prof, I discovered something weird -- all the view templates in
all the plugins were being recompiled -- EVERY REQUEST.

This was adding a whole second onto the amount of time required to
process a request (obviously, if you don't have view templates in your
plugins you won't have a problem here -- we have 247). So 100
requests takes at least 100 seconds to process, doing 24700 template
compiles. This is basically completely CPU-bound, which explains why
our servers were exploding under reasonably small loads.

It seems that the way the view paths from the plugins were being added
was wrong; basically they're just added temporarily to the view paths,
so every request re-adds them. The problem with this is that when the
eager_loading of templates is turned on (i.e. cache_classes = true),
this recompiles all the templates in the path. So that happens every
request.

In our desert_extras plugin we use to add a couple of features that
are missing (that we used from Engines), I've put a monkeypatch to
undo the change to ActionView::Base, and to load the view paths at the
correct time (in the Rails Initializer).

Code here:
http://code.moneyspyder.net/desert_extras/trunk/lib/desert_extras/fix_view_loading_bug.rb

We've gone from our staging slice being able to handle 66 requests/
minute to around 1100. So this is a patch worth applying.

Simon Russell
Moneyspyder

[Andrei Erdoss]

Is this going to be added to official Desert repo? It would be great to have it.

--
Andrei Erdoss

[Simon Russell]

Hopefully, that was my intention -- I'm not sure if anyone else has
experienced the bug though? Maybe it was just made worse for us
because we have so many templates in our plugins?

[Brian Takita]

On Sat, May 16, 2009 at 10:54 AM, Simon Russell <si...@bellyphant.com> wrote:
>
> Hopefully, that was my intention -- I'm not sure if anyone else has
> experienced the bug though?  Maybe it was just made worse for us
> because we have so many templates in our plugins?

We will be adding this to Desert. I'll go ahead and integrate it today
or tomorrow.

We are also considering simplifying how Desert's Rails integration.
This can be done by having the master branch support the latest
version of Rails only.
Other branches will be created to support previous versions of Rails.
The upside is that it would be easier to utilize whatever Rails
provides (e.g. Engines in Rails 2.3.2) and it would remove all of that
awful version branching logic in Desert.

Such a scheme would make it a bit more difficult to find the correct
version, but that can be mitigated by good error messages.

[Simon Russell]

I always thought the multiple Rails version support was kind of
impressive :) (And actually coded in a basically nice way.) One of
the things about Engines that annoyed me was that updates were almost
always bound to the latest version of Rails.

It wouldn't be an entirely bad plan though, people on old versions of
Rails can just use an old gem. It would also probably simplify things
a bit to just get rid of Rails 1.x support. And potentially get rid
of bugfix release support (i.e. 2.2.2 vs 2.2.0) -- so just allow
latest 2.2 and 2.3 or something.

[Brian Takita]

On Sat, May 16, 2009 at 1:56 PM, Simon Russell <si...@bellyphant.com> wrote:
>
> I always thought the multiple Rails version support was kind of
> impressive :)  (And actually coded in a basically nice way.)  One of
> the things about Engines that annoyed me was that updates were almost
> always bound to the latest version of Rails.
>
> It wouldn't be an entirely bad plan though, people on old versions of
> Rails can just use an old gem.  It would also probably simplify things
> a bit to just get rid of Rails 1.x support.  And potentially get rid
> of bugfix release support (i.e. 2.2.2 vs 2.2.0) -- so just allow
> latest 2.2 and 2.3 or something.

The approach that we are trying on Erector is to have branches to
support an earlier version of Rails.
This means that if there is a new change to Desert that affects all
versions of Rails support, then these changes can be merged into all
of the branches.

So, we can probably have a versioning scheme like:
2.3.2.5 (Rails 2.3.2 with Desert version 5)
2.2.0.5 (Rails 2.2.0 with Desert version 5)

The easiest thing to do would be to take a poll of who still uses
Desert with which versions of Rails. We can then commit to releasing
gems for those versions of Rails.
If there is an "unsupported" version of Rails, then there should be
enough infrastructure (which is not yet built in) to create a branch
and release a gem.

[Simon Russell]

That sounds like a decent plan to me, I'd be happy with that. (We use
2.2.2, but will upgrade to 2.3 at some point in the next few months
I'd say.)

[Alberto Molpeceres]

For me sounds fine too.

We had a fork of desert for our Rails 2.2 work, so I suppose Rails
2.3.2 is enough for us.

--
Alberto Molpeceres.
Linking Paths.
Find us at http://www.linkingpaths.com

[Andrei Erdoss]

Sounds good. We are interested in the Rails 2.3.2 version.

--
Andrei Erdoss