[PATCH] preload_app can take an optional block for warmup

[PATCH] preload_app can take an optional block for warmup

[Aman Gupta]

---
 lib/unicorn/configurator.rb    | 19 ++++++++++++++++---
 lib/unicorn/http_server.rb     |  3 +++
 test/unit/test_configurator.rb |  8 ++++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 0d0eac7..a0ae576 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -441,9 +441,22 @@ class Unicorn::Configurator
   # by properly deploying all required code and dependencies.
   # Using preload_app=true means any application load error will
   # cause the master process to exit with an error.
-
-  def preload_app(bool)
-    set_bool(:preload_app, bool)
+  #
+  # preload_app can also take an optional block. This block will be invoked
+  # with the rack application and can be used to "warm up" the application
+  # before deployment:
+  #
+  #  preload_app do |app|
+  #    client = Rack::MockRequest.new(app)
+  #    client.get('/')
+  #  end
+  #
+  def preload_app(bool=nil, &block)
+    if block_given? || bool.respond_to?(:call)
+      set_hook(:preload_app, block_given? ? block : bool, 1)
+    else
+      set_bool(:preload_app, bool)
+    end
   end
 
   # Toggles making \env[\"rack.input\"] rewindable.
diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index bed24d0..d749a92 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -721,6 +721,9 @@ class Unicorn::HttpServer
         Gem.refresh
       end
       self.app = app.call
+      if preload_app.respond_to?(:call)
+        preload_app[app]
+      end
     end
   end
 
diff --git a/test/unit/test_configurator.rb b/test/unit/test_configurator.rb
index 1298f0e..8a1a68e 100644
--- a/test/unit/test_configurator.rb
+++ b/test/unit/test_configurator.rb
@@ -172,4 +172,12 @@ class TestConfigurator < Test::Unit::TestCase
     end
   end
 
+  def test_preload_app
+    test_struct = TestStruct.new
+    [ true, false, proc { |a| }, Proc.new { |a| }, lambda { |a| } ].each do |my_proc|
+      Unicorn::Configurator.new(:preload_app => my_proc).commit!(test_struct)
+      assert_equal my_proc, test_struct.preload_app
+    end
+  end
+
 end
-- 
1.8.3.4

_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] preload_app can take an optional block for warmup

[Eric Wong]

Aman Gupta <aman@tmm1.net> wrote:

Thanks for the patch!

I expect a commit message body to describe why it is useful.

In particular, what benefit does this have over putting the same
code in config.ru or config/initializer.rb (or similar?)

For user-visible config changes like these, it can be similar/identical
to the added documentation.

Anyways, I agree warming up the app is often necessary, but I'm not
convinced it's necessary change unicorn for it.  It makes sense
to warmup apps on servers other than unicorn, too.

> --- a/lib/unicorn/http_server.rb
> +++ b/lib/unicorn/http_server.rb
> @@ -721,6 +721,9 @@ class Unicorn::HttpServer
>          Gem.refresh
>        end
>        self.app = app.call
> +      if preload_app.respond_to?(:call)
> +        preload_app[app]
> +      end

Since you're testing for respond_to?(:call), it should be less confusing
to use "preload_app.call(app)" here instead of "preload_app[app]"
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] preload_app can take an optional block for warmup

[Aman Gupta]

> In particular, what benefit does this have over putting the same
> code in config.ru or config/initializer.rb (or similar?)

With my patch, preload_app yields a rack app object which includes the
middleware stack. AFAIK there's no way to do this in the context of
config.ru, since the app is still being built. My goal is to warm up
the final application that will be serving traffic, including all
middleware.

On Sat, Sep 21, 2013 at 1:49 AM, Eric Wong <normalperson@yhbt.net> wrote:
> Aman Gupta <aman@tmm1.net> wrote:
>
> Thanks for the patch!
>
> I expect a commit message body to describe why it is useful.
>
> In particular, what benefit does this have over putting the same
> code in config.ru or config/initializer.rb (or similar?)
>
> For user-visible config changes like these, it can be similar/identical
> to the added documentation.
>
> Anyways, I agree warming up the app is often necessary, but I'm not
> convinced it's necessary change unicorn for it.  It makes sense
> to warmup apps on servers other than unicorn, too.
>
>> --- a/lib/unicorn/http_server.rb
>> +++ b/lib/unicorn/http_server.rb
>> @@ -721,6 +721,9 @@ class Unicorn::HttpServer
>>          Gem.refresh
>>        end
>>        self.app = app.call
>> +      if preload_app.respond_to?(:call)
>> +        preload_app[app]
>> +      end
>
> Since you're testing for respond_to?(:call), it should be less confusing
> to use "preload_app.call(app)" here instead of "preload_app[app]"
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying

Re: [PATCH] preload_app can take an optional block for warmup

[Eric Wong]

Aman Gupta <aman@tmm1.net> wrote:
> > In particular, what benefit does this have over putting the same
> > code in config.ru or config/initializer.rb (or similar?)
> 
> With my patch, preload_app yields a rack app object which includes the
> middleware stack. AFAIK there's no way to do this in the context of
> config.ru, since the app is still being built. My goal is to warm up
> the final application that will be serving traffic, including all
> middleware.

Good point.  Perhaps this can be triggered via Rack::Builder#to_app
instead, so it can benefit _all_ Rack HTTP servers.

Care to move this discussion over to rack-devel?

There's the odd non-Rack::Builder case, too, but I think everybody just
uses Rack::Builder via config.ru, right?
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying