logstash-plugins / logstash-output-lumberjack

Apache License 2.0
6 stars 24 forks source link

Fix drop events between two logstash instances #7

Closed ph closed 9 years ago

ph commented 9 years ago

This PR introduce the bulk send feature of the ruby client and add a local buffer inside this plugin. Theses changes make the plugin closer to the LSF behavior.

ph commented 9 years ago

@jsvd Can you test this? you need latest version of https://github.com/elastic/ruby-lumberjack/pull/9

jordansissel commented 9 years ago

@ph for clarification, this PR requires https://github.com/elastic/ruby-lumberjack/pull/9 be released first, right?

ph commented 9 years ago

@jordansissel yes, https://github.com/elastic/ruby-lumberjack/pull/9 add support for bulk send which this PR need

ph commented 9 years ago

code review done

ph commented 9 years ago

@jordansissel Can I get a LGTM for this one? I'll squash after.

jordansissel commented 9 years ago

@ph will review shortly <3

jordansissel commented 9 years ago

Code + Test code looks good, will test shortly.

jordansissel commented 9 years ago

Can't build the gem:

ERROR:  While executing gem ... (Gem::InvalidSpecificationException)
    duplicate dependency on stud (>= 0, development), (>= 0) use:
    add_runtime_dependency 'stud', '>= 0', '>= 0'
jordansissel commented 9 years ago

Confirmed tests passing for me

jordansissel commented 9 years ago

Tested perf is 2x (eyeballing it with pv -War) with this patch (3k/sec -> 6k/sec). We can probably improve further but this is a lovely start :)

elasticsearch-bot commented 9 years ago

Merged sucessfully into master!