Amqp driver select #110

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

Contributor
lusis commented Feb 12, 2012

This adds support selectable drivers for AMQP.

Currently it's pulling in hot_bunnies via git. Obviously not a solid long-term solution.

Note that under JRuby, hot_bunnies outperforms bunny by a serious stretch.

Contributor

still relevant?

@fetep fetep commented on the diff Jun 18, 2012
@@ -2,6 +2,7 @@ source :rubygems
gem "cabin", "0.3.8" # for logging. apache 2 license
gem "bunny" # for amqp support, MIT-style license
+gem "hot_bunnies", :git => "git://github.com/ruby-amqp/hot_bunnies.git", :platforms => :jruby #License: MIT
fetep
fetep Jun 18, 2012 Contributor

so this is going to pull from master every time we build? is there a way to lock this down to a tag/specific commit/something? it looks like this gem is released now, too, so maybe we don't need :git?

@fetep fetep commented on the diff Jun 18, 2012
lib/logstash/inputs/amqp.rb
@@ -72,6 +74,16 @@ class LogStash::Inputs::Amqp < LogStash::Inputs::Base
# Validate SSL certificate
config :verify_ssl, :validate => :boolean, :default => false
+ # Driver selection
+ # By default, logstash will use the `hot_bunnies` gem under JRuby
+ # and the `bunny` gem under MRI/YARV variants
+ # If you need to explcitly set this, do so here
fetep
fetep Jun 18, 2012 Contributor

spelling: s/explcitly/explicitly/

@fetep fetep commented on the diff Jun 18, 2012
lib/logstash/inputs/amqp.rb
timer = @metric_amqp_read.time
- @queue.subscribe({:ack => @ack}) do |data|
- timer.stop
- e = to_event(data[:payload], @amqpurl)
- if e
- @metric_queue_write.time do
- queue << e
+ if @driver == 'hot_bunnies'
+ subscription = @queue.subscribe(:ack => @ack, :blocking => true) do |headers,data|
+ timer.stop
+ e = to_event(data, @amqp_url)
+ if e
+ @metric_queue_write.time do
+ queue << e
+ headers.ack if @ack == true # ack after we know we're good
fetep
fetep Jun 18, 2012 Contributor

is this ack the only difference between hot_bunnies and the "else" code? If so, maybe we can just wrap this line in an if?

Contributor
lusis commented Jun 18, 2012

woops. Sorry I missed the comments.

Let's close this for now. I'll keep my fork around as a testbed and try a cleaner merge down the road. We should at least bump to the latest bunny gem as well to make sure we're getting the most bang for the buck.

@lusis lusis closed this Jun 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment