logstash-plugins / logstash-output-file

Apache License 2.0
23 stars 53 forks source link

flush_interval > 0 not working #62

Closed heli80 closed 6 years ago

heli80 commented 6 years ago

Hi!

Output plugin always flushes on each message (even with flush_interval > 0) since there is a small typo/bug on this line: https://github.com/logstash-plugins/logstash-output-file/blob/master/lib/logstash/outputs/file.rb#L144

It should correctly call flush(fd) instead of fd.flush

Br, Heli

marioplumbarius commented 6 years ago

@heli80 how often are you writing?

heli80 commented 6 years ago

Ho often? I would like to handle high volume logs (a line every some ms), where flushing only every x seconds would bring a significant (batch) performance gain on the storage side.

marioplumbarius commented 6 years ago

I tried to reproduce the hebaviour you've encountered, but it only happened when the rate of writes is small (which should not be a problem, given you're going to write multiple times in a single second).

Have you tried writing a test case to reproduce the problem you're facing with high write rate?

Can you share the config (hide confidential stuff), so others can help as well?

heli80 commented 6 years ago

If you take a look at the code it's pretty obvious that the private method

private
def flush(fd)

should be called instead of

fd.flush

which does an unconditional flush on the file descriptor.

The private method flush(fd) would otherwise never be called at all!

flush(fd) internally has the necessary logic to defer fd.flush with flush_interval > 0 as it's supposed to be.

Wouldn't you agree?

heli80 commented 6 years ago

Any update on this? The fix is really trivial... Br

jsvd commented 6 years ago

@heli80 do you mind creating a PR? I'll review and merge asap

jsvd commented 6 years ago

version 4.2.4 has been published with the fix from #67. Thanks to @heli80 for the patience and PR.