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

add simple sniffer for simple proxy/lb cases #459

Merged
merged 1 commit into from Aug 11, 2018

Conversation

richm
Copy link
Contributor

@richm richm commented Aug 10, 2018

When going through a proxy or load balancer to Elasticsearch,
do not try to use the _nodes discovery mechanism - instead, just
assume the given host(s) are the only hostnames to use.
This is useful e.g. in the case of Kubernetes going through an
Elasticsearch Service, where you want to periodically reconnect
the long-lived http connections to evenly spread the load throughout
the cluster.

(check all that apply)

  • tests added
  • tests passing
  • README updated (if needed)
  • README Table of Contents updated (if needed)
  • History.md and version in gemspec are untouched
  • backward compatible
  • feature works in elasticsearch_dynamic (not required but recommended)

@richm
Copy link
Contributor Author

richm commented Aug 10, 2018

@jcantrill @ewolinetz @nhosoi

Copy link
Collaborator

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

Looks good to me except a nit comment.

@@ -86,6 +86,8 @@ def initialize(retry_stream)
config_param :pipeline, :string, :default => nil
config_param :with_transporter_log, :bool, :default => false
config_param :emit_error_for_missing_id, :bool, :default => false
config_param :sniffer_class_name, :string, :default => nil
config_param :reload_after, :integer, :default => -1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a new constant for reload_after and use it?

DEFAULT_RELOAD_AFTER = -1

#...

config_param :reload_after, :integer, :default => DEFAULT_RELOAD_AFTER

# ...

if local_reload_connections && @reload_after > DEFAULT_RELOAD_AFTER

is better.

When going through a proxy or load balancer to Elasticsearch,
do not try to use the _nodes discovery mechanism - instead, just
assume the given host(s) are the only hostnames to use.
This is useful e.g. in the case of Kubernetes going through an
Elasticsearch Service, where you want to periodically reconnect
the long-lived http connections to evenly spread the load throughout
the cluster.
@ewolinetz
Copy link
Contributor

LGTM

Copy link
Collaborator

@cosmo0920 cosmo0920 left a comment

Choose a reason for hiding this comment

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

LGTM!

@cosmo0920 cosmo0920 merged commit 313336b into uken:v0.12 Aug 11, 2018
cosmo0920 added a commit that referenced this pull request Aug 12, 2018
add simple sniffer for simple proxy/lb cases

Co-Authored-By: Rich Megginson <rmeggins@redhat.com>
cosmo0920 added a commit that referenced this pull request Aug 12, 2018
add simple sniffer for simple proxy/lb cases

Co-Authored-By: Rich Megginson <rmeggins@redhat.com>
cosmo0920 added a commit that referenced this pull request Aug 13, 2018
Merge pull request #459 from richm/v0.12-simple-sniffer
richm added a commit to richm/origin-aggregated-logging that referenced this pull request Aug 14, 2018
…ing-mux or elasticsearch to help balance sessions

https://bugzilla.redhat.com/show_bug.cgi?id=1489533

uken/fluent-plugin-elasticsearch#459
implements support for reloading connections when the
Elasticsearch is behind a proxy/load balancer, as in our case,
and allows specifying the reload interval in terms of the
number of operations.

This PR adds support for the following env. vars which can be
set in the fluentd daemonset/mux deployment.  The ability to
set these is provided primarily for experimentation, not something
which will ordinarily require tuning in production.
`ES_RELOAD_CONNECTIONS` - boolean - default `true`
`ES_RELOAD_AFTER` - integer - default `100`
`ES_SNIFFER_CLASS_NAME` - string - default `Fluent::Plugin::ElasticsearchSimpleSniffer`
There are also `OPS_` named env. vars which will override the
corresponding `ES_` named env. var.

That is, by default, fluentd will reload connections to
Elasticsearch every 100 operations (NOTE: not every 100 records!)
These include internal `ping` operations, so will not exactly
correspond to each bulk index request.
richm added a commit to richm/origin-aggregated-logging that referenced this pull request Aug 14, 2018
…ing-mux or elasticsearch to help balance sessions

https://bugzilla.redhat.com/show_bug.cgi?id=1489533

uken/fluent-plugin-elasticsearch#459
implements support for reloading connections when the
Elasticsearch is behind a proxy/load balancer, as in our case,
and allows specifying the reload interval in terms of the
number of operations.

This PR adds support for the following env. vars which can be
set in the fluentd daemonset/mux deployment.  The ability to
set these is provided primarily for experimentation, not something
which will ordinarily require tuning in production.
`ES_RELOAD_CONNECTIONS` - boolean - default `true`
`ES_RELOAD_AFTER` - integer - default `100`
`ES_SNIFFER_CLASS_NAME` - string - default `Fluent::Plugin::ElasticsearchSimpleSniffer`
There are also `OPS_` named env. vars which will override the
corresponding `ES_` named env. var.

That is, by default, fluentd will reload connections to
Elasticsearch every 100 operations (NOTE: not every 100 records!)
These include internal `ping` operations, so will not exactly
correspond to each bulk index request.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/origin-aggregated-logging that referenced this pull request Aug 15, 2018
…ing-mux or elasticsearch to help balance sessions

https://bugzilla.redhat.com/show_bug.cgi?id=1489533

uken/fluent-plugin-elasticsearch#459
implements support for reloading connections when the
Elasticsearch is behind a proxy/load balancer, as in our case,
and allows specifying the reload interval in terms of the
number of operations.

This PR adds support for the following env. vars which can be
set in the fluentd daemonset/mux deployment.  The ability to
set these is provided primarily for experimentation, not something
which will ordinarily require tuning in production.
`ES_RELOAD_CONNECTIONS` - boolean - default `true`
`ES_RELOAD_AFTER` - integer - default `100`
`ES_SNIFFER_CLASS_NAME` - string - default `Fluent::Plugin::ElasticsearchSimpleSniffer`
There are also `OPS_` named env. vars which will override the
corresponding `ES_` named env. var.

That is, by default, fluentd will reload connections to
Elasticsearch every 100 operations (NOTE: not every 100 records!)
These include internal `ping` operations, so will not exactly
correspond to each bulk index request.
richm added a commit to richm/origin-aggregated-logging that referenced this pull request Aug 15, 2018
…ing-mux or elasticsearch to help balance sessions

3.9 bz - https://bugzilla.redhat.com/show_bug.cgi?id=1616354

uken/fluent-plugin-elasticsearch#459
implements support for reloading connections when the
Elasticsearch is behind a proxy/load balancer, as in our case,
and allows specifying the reload interval in terms of the
number of operations.

This PR adds support for the following env. vars which can be
set in the fluentd daemonset/mux deployment.  The ability to
set these is provided primarily for experimentation, not something
which will ordinarily require tuning in production.
`ES_RELOAD_CONNECTIONS` - boolean - default `true`
`ES_RELOAD_AFTER` - integer - default `100`
`ES_SNIFFER_CLASS_NAME` - string - default `Fluent::Plugin::ElasticsearchSimpleSniffer`
There are also `OPS_` named env. vars which will override the
corresponding `ES_` named env. var.

That is, by default, fluentd will reload connections to
Elasticsearch every 100 operations (NOTE: not every 100 records!)
These include internal `ping` operations, so will not exactly
correspond to each bulk index request.

(cherry picked from commit f084fee)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants