Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

output: Work retry_forever and <secondary> together. fix #2257 #2276

Merged
merged 1 commit into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/fluent/plugin/output.rb
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,9 @@ def configure(conf)
raise Fluent::ConfigError, "Invalid <secondary> section for non-buffered plugin" unless @buffering
raise Fluent::ConfigError, "<secondary> section cannot have <buffer> section" if @secondary_config.buffer
raise Fluent::ConfigError, "<secondary> section cannot have <secondary> section" if @secondary_config.secondary
raise Fluent::ConfigError, "<secondary> section and 'retry_forever' are exclusive" if @buffer_config.retry_forever
if @buffer_config.retry_forever
log.warn "<secondary> with 'retry_forever', only unrecoverable errors are moved to secondary"
end

secondary_type = @secondary_config[:@type]
unless secondary_type
Expand Down
8 changes: 2 additions & 6 deletions lib/fluent/plugin_helper/retry_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ def initialize(title, wait, timeout, forever, max_steps, randomize, randomize_wi
@randomize = randomize
@randomize_width = randomize_width

if forever && secondary
raise "BUG: forever and secondary are exclusive to each other"
end

@forever = forever
@max_steps = max_steps

Expand Down Expand Up @@ -118,12 +114,12 @@ def naive_next_time(retry_times)
end

def secondary?
@secondary && (@current == :secondary || current_time >= @secondary_transition_at)
!@forever && @secondary && (@current == :secondary || current_time >= @secondary_transition_at)
end

def step
@steps += 1
if @secondary && @current != :secondary && current_time >= @secondary_transition_at
if !@forever && @secondary && @current != :secondary && current_time >= @secondary_transition_at
@current = :secondary
@secondary_transition_steps = @steps
end
Expand Down
12 changes: 8 additions & 4 deletions test/plugin/test_output_as_buffered_retries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -706,16 +706,20 @@ def get_log_time(msg, logs)
end

sub_test_case 'buffered output configured as retry_forever' do
test 'configuration error will be raised if secondary section is configured' do
setup do
Fluent::Plugin.register_output('output_retries_secondary_test', FluentPluginOutputAsBufferedRetryTest::DummyFullFeatureOutput2)
end

test 'warning logs are generated if secondary section is configured' do
chunk_key = 'tag'
hash = {
'retry_forever' => true,
'retry_randomize' => false,
}
i = create_output()
assert_raise Fluent::ConfigError do
i.configure(config_element('ROOT','',{},[config_element('buffer',chunk_key,hash),config_element('secondary','')]))
end
i.configure(config_element('ROOT','',{},[config_element('buffer',chunk_key,hash),config_element('secondary','', {'@type' => 'output_retries_secondary_test'})]))
logs = i.log.out.logs
assert { logs.any? { |l| l.include?("<secondary> with 'retry_forever', only unrecoverable errors are moved to secondary") } }
end

test 'retry_timeout and retry_max_times will be ignored if retry_forever is true for exponential backoff' do
Expand Down