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

Fix race condition of retry state in flush thread #1623

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

repeatedly
Copy link
Member

@repeatedly repeatedly commented Jul 11, 2017

This is simple solution but performance is bit decreased.
The better solution is changing retry mechanizm, e.g. don't share state, but it makes
the code complicated. This is feature task.

This fixes #1555

This is simple solution but performance is bit decreased.
The better solution is changing retry mechanizm but it makes
the code complicated. This is feature task.
@repeatedly repeatedly requested a review from mururu July 11, 2017 23:36
@repeatedly repeatedly self-assigned this Jul 11, 2017
@repeatedly repeatedly added bug Something isn't working v0.14 labels Jul 11, 2017
@repeatedly
Copy link
Member Author

Reproduce step:

  • conf
# dummy plugin is also ok. Need to generate lots of events.
<source>
  @type tail
  path /pat/to/log
  tag test.tail
  read_from_head true
  <parse>
    @type none
  </parse>
</source>

<match test.**>
  @type buf_test
  @id buf_output

  <buffer>
    chunk_limit_size 500000
    flush_interval 5s
    flush_thread_count 16
    #retry_max_times 15
    retry_max_interval 2s
  </buffer>
</match>
  • out_buf_test.rb
require 'fluent/output'

module Fluent::Plugin
  class BufTestOutput < Output
    Fluent::Plugin.register_output('buf_test', self)

    config_section :buffer do
      config_set_default :chunk_keys, ["tag"]
    end

    def initialize
      super
      @num = 0
      @m = Mutex.new
    end

    def configure(conf)
      super
    end

    def multi_workers_ready?
      true
    end

    def start
      super
    end

    def close
      log.error @num
      super
    end

    def write(chunk)
      if rand(10) > 7
        raise
      end

      @m.synchronize {
        @num += chunk.size
      }
    end
  end
end

@repeatedly
Copy link
Member Author

I forgot to send a patch...

@mururu Could you check this patch?

Copy link
Member

@mururu mururu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
And I confirmed that I can reproduce the bug using the given conf (but I used in_dummy) and it disappears on this patch.

@repeatedly repeatedly merged commit b3e5e94 into master Jul 12, 2017
@repeatedly repeatedly deleted the fix-race-condition-in-flush-thread branch July 12, 2017 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v0.14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error_class=NoMethodError error="undefined method `next_time' for nil:NilClass"
2 participants