Improve flush_interval handling

[repeatedly]

This patch improves flush_interval parameter handing.

• Set flush_mode interval when v0.12 configuration has flush_interval

This avoids a trouble for existing out_file and out_s3 configurations

• Log warning message for flush_interval without flush_mode interval in <buffer>

Currenlty, flush_interval is ignored silently with following configuration. It is hard to debug for users.

<buffer time>
  timekey 3600
  flush_interval 30s
</buffer>

[repeatedly]

@tagomoris Could you check this patch?

[tagomoris]

It looks better to set :interval when flush_mode is :default, even when timekey is specified, here:
https://github.com/fluent/fluentd/blob/master/lib/fluent/plugin/output.rb#L292
We need to know flush_interval is set in <buffer> section or not before setting :interval... how about to add has_flush_interval before super as same with has_buffer_section?

[tagomoris]

And it's better to warn when both of flush_interval and flush_mode lazy(or immediate) are configured explicitly by users (these are exclusive parameters).

[repeatedly]

It looks better to set :interval when flush_mode is :default

https://github.com/fluent/fluentd/blob/master/lib/fluent/plugin/output.rb#L293

From above line, :default is changed to :lazy or :interval so :default is not appeared here, right?

how about to add has_flush_interval before super as same with has_buffer_section?

It seems good.

these are exclusive parameters

If so, should we raise an ConfigError when flush_interval and flush_mode lazy are set explictly?

[tagomoris]

I mean to set @flush_mode = :interval at L293 when flush_interval is explicitly specified in configuration even if @chunk_key_time is true.

[tagomoris]

If we doesn't take care about compatibility layer, it's better to raise configuration errors when both of flush_mode lazy and flush_interval are specified in configuration file.
But compatibility layer may specify these both if a plugin is TimeSlicedOutput and flush_interval exists in configuration. So we can't raise errors for such cases. warn is better.

[repeatedly]

flush_interval is explicitly specified in configuration even if @chunk_key_time is true.

This means we should use :interval with following configuration?

<buffer>
  flush_interval 10s # use flush_mode interval
</buffer>

And warning for follwing configuration?

<buffer>
  flush_interval 10s
  flush_mode lazy
</buffer>

[tagomoris]

1st example: No. It depends on default of plugins, and output.rb should warn about ignored flush_interval when plugins' default is not interval.

My first point is that we cannot know the :interval set in @flush_mode is from plugin default or configuraitons at L314 (because it can be set at L293). So we should check conf before super to show appropriate warning.

2nd example: No. IMO it's better to raise error for inconsistent configuration.

The last my comment was wrong, because Fluent::Compat::TimeSlicedOutput#configure will set flush_mode as interval if conf has flush_interval. So we can raise exception if both of flush_mode lazy and flush_interval ... are specified in configurations explicitly.

[tagomoris]

@repeatedly any updates?

[repeatedly]

Applied reviews.

[repeatedly]

@tagomoris could you check again?

[tagomoris]

LGTM