Merge pull request #2276 from fluent/support-retry_forever-and-secondary

output: Work retry_forever and <secondary> together. fix #2257
fluent · Feb 1, 2019 · 407f674 · 407f674
2 parents b2d04fd + e71bb8c
commit 407f674
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 11 deletions.
diff --git a/lib/fluent/plugin/output.rb b/lib/fluent/plugin/output.rb
@@ -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

diff --git a/lib/fluent/plugin_helper/retry_state.rb b/lib/fluent/plugin_helper/retry_state.rb
@@ -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
 
@@ -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

diff --git a/test/plugin/test_output_as_buffered_retries.rb b/test/plugin/test_output_as_buffered_retries.rb
@@ -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