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

Use better default values for workers/pools/buffers #1512

Closed
6 tasks done
jordansissel opened this issue Jul 8, 2014 · 21 comments
Closed
6 tasks done

Use better default values for workers/pools/buffers #1512

jordansissel opened this issue Jul 8, 2014 · 21 comments

Comments

@jordansissel
Copy link
Contributor

Things to tune and/or make default:

Specific to plugins:

  • redis: use batch operations by default.
  • enable batch operations by default where possible

Specific Issues:

Will be updated as we file more individual issues against plugins

@kobazik
Copy link

kobazik commented Jul 8, 2014

+1

@jordansissel
Copy link
Contributor Author

Wanted to use Elasticsearch's bundled Sigar libs, but the build process removes them. Oldest commit I can find for why I did this is here: c78d1e3 and I haven't a clue why I did it ;)

Easy to fix. ;)

jordansissel added a commit to jordansissel/logstash that referenced this issue Jul 8, 2014
We piggyback on Elasticsearch's use of Sigar to get the system info.

WIP for elastic#1512
jordansissel added a commit to jordansissel/logstash that referenced this issue Jul 8, 2014
If cpu core info isn't available, the default (and previous
behavior) is 1.

WIP elastic#1512
@colinsurprenant
Copy link
Contributor

🎶 Come in here, dear boy, have a Sigar.
You're gonna go far, you're gonna fly high,
You're never gonna die, you're gonna make it if you try; they're gonna love you. 🎶

@colinsurprenant colinsurprenant added this to the v1.5.0 milestone Jul 18, 2014
jordansissel added a commit to jordansissel/logstash that referenced this issue Jul 30, 2014
We piggyback on Elasticsearch's use of Sigar to get the system info.

WIP for elastic#1512
jordansissel added a commit to jordansissel/logstash that referenced this issue Jul 30, 2014
If cpu core info isn't available, the default (and previous
behavior) is 1.

WIP elastic#1512
@jlintz
Copy link

jlintz commented Aug 23, 2014

@jordansissel given that some filters aren't thread safe (multiline), I wonder if it makes sense to check if any such filters are configured before setting the workers = # cpu cores?

@jordansissel
Copy link
Contributor Author

Indeed! Part of this work will be to detect that and act accordingly.

@suyograo suyograo removed this from the v1.5.0 milestone Nov 6, 2014
@suyograo suyograo added the v2.0.0 label Nov 6, 2014
@suyograo suyograo mentioned this issue Jul 14, 2015
4 tasks
@suyograo suyograo assigned guyboertje and unassigned jordansissel Aug 7, 2015
@guyboertje
Copy link
Contributor

I want to begin a discussion about which 'values' are considered default and important.

Please refer to this overview meta issue

@guyboertje
Copy link
Contributor

pipeline.rb

  • line 36 SizedQueue.new(20)
  • line 44 @settings = { "filter-workers" => 1 }
  • line 165 Thread.new { Stud.interval(5) { @input_to_filter.push(LogStash::FLUSH) } }

Action for line 44: Add support for filter_workers default to cpu-cores

@guyboertje
Copy link
Contributor

agent.rb

  • line 22 option ["-w", "--filterworkers"], "COUNT", default = 1
  • line 26 option "--watchdog-timeout", "SECONDS", default = 10
  • line 131 Thread.new(@logger) {|logger| sleep 5; logger.warn(I18n.t("logstash.agent.slow_shutdown"))

Action for line 22: Add support for filter_workers default to cpu-cores

Action for line 26: Remove Watchdog in the code base

@guyboertje
Copy link
Contributor

/outputs/base.rb

  • line 71 @worker_queue = SizedQueue.new(20)

[suyog] I would leave this as-is

@ph
Copy link
Contributor

ph commented Sep 1, 2015

The current implementation of the watchdog will go alway, I'll make a PR to remove it completely.
It's not currently used and it was a source of problems for some plugins DNS IIRC.

@suyograo
Copy link
Contributor

suyograo commented Sep 2, 2015

@guyboertje @settings = { "filter-workers" => 1 } is definitely candidate for changing. IMO we should default that to # of cores in the system. But we need to be careful of one usecase: multiline filter which needs events to be in order (and will not work with -w). But, we can just check for existence of that filter at start up and not use # of cores

@guyboertje
Copy link
Contributor

@suyograo - there are checks for in-order events AFAIK.

@guyboertje
Copy link
Contributor

inputs/elasticsearch.rb

  • line 50 config :size, :validate => :number, :default => 1000
  • line 55 config :scroll, :validate => :string, :default => "1m"

@guyboertje
Copy link
Contributor

inputs/http.rb

  • line 50 config :threads, :validate => :number, :default => 4 should be no. of cpus

@guyboertje
Copy link
Contributor

inputs/imap.rb

  • line 26 config :fetch_count, :validate => :number, :default => 50
  • line 28 config :check_interval, :validate => :number, :default => 300

@guyboertje
Copy link
Contributor

inputs/kafka.rb

  • line 56 config :consumer_threads, :validate => :number, :default => 1 should this be called 'partitions'?
  • line 58 config :queue_size, :validate => :number, :default => 20
  • line 63 config :rebalance_max_retries, :validate => :number, :default => 4
  • line 65 config :rebalance_backoff_ms, :validate => :number, :default => 2000

@guyboertje
Copy link
Contributor

inputs/lumberjack.rb

  • line 40 BUFFERED_QUEUE_SIZE = 1
  • line 41 RECONNECT_BACKOFF_SLEEP = 0.5

@guyboertje
Copy link
Contributor

inputs/rabbitmq.rb

  • line 76 config :prefetch_count, :validate => :number, :default => 256

@guyboertje
Copy link
Contributor

inputs/redis.rb

  • line 59 config :batch_count, :validate => :number, :default => 1 should this start at queue size (20) and, possibly, dynamically change to grab as many as would fill the queue?

Action for line 59

@guyboertje
Copy link
Contributor

NOTE: Magic numbers/strings, just convert the magic value into a descriptive method call from LogStash::Util::Defaults

@purbon
Copy link
Contributor

purbon commented Nov 3, 2015

done the timestamp behaviour at -> logstash-plugins/logstash-filter-date#36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants