Connection reload threadsafety + more efficient manticore pooling #281
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewvc
force-pushed
the
connection_reload_theradsafety
branch
from
81cdc4c to
f8a7655
Compare
andrewvc
changed the title
| info.merge :host => host, :port => port, :id => id | ||
| end | ||
| end.compact | ||
| # We use a timeout in the client itself here because using a library like |
There was a problem hiding this comment.
Let's put this comment into the commit message instead of the code?
|
I understand and like the motivation, but I think on the implementation layer we have to solve the problem with the timeout, as evident from the failing test and our Slack chat. If the base class needs additions, let's do them. |
andrewvc
force-pushed
the
connection_reload_theradsafety
branch
from
f8a7655 to
8a1086c
Compare
|
@karmi as we discussed I removed the timeout issue. There is one failing test, but I don't test its legitimate, what do you think? |
andrewvc
added a commit
to andrewvc/logstash-output-elasticsearch
that referenced
this pull request
Mar 2, 2016
Fixes logstash-plugins#377 Before acceptance this will need elastic/elasticsearch-ruby#281 Merged in and used for the ruby client
andrewvc
added a commit
to andrewvc/logstash-output-elasticsearch
that referenced
this pull request
Mar 2, 2016
Fixes logstash-plugins#377 Before acceptance this will need elastic/elasticsearch-ruby#281 Merged in and used for the ruby client
andrewvc
added a commit
to andrewvc/logstash-output-elasticsearch
that referenced
this pull request
Mar 2, 2016
Fixes logstash-plugins#377 Before acceptance this will need elastic/elasticsearch-ruby#281 Merged in and used for the ruby client
karmi
pushed a commit
that referenced
this pull request
Mar 3, 2016
|
I've split the commit into two separate ones (one for the threadsafety, one for the Manticore optimization), and merged into master. When you verify this does what is intended in Logstash, @andrewvc, we can do a release later in the day. |
andrewvc
added a commit
to logstash-plugins/logstash-output-elasticsearch
that referenced
this pull request
May 3, 2016
Fixes #377 Before acceptance this will need elastic/elasticsearch-ruby#281 Merged in and used for the ruby client We no longer need the concurrent ruby gem Support manticore pool options directly Add sniffing to info log level Proper info logging for sniffing Exponential backoff for retries Make this actually multithreaded by omitting request mutex More fixes for threadsafety Add healthcheck option Checkpoint
andrewvc
added a commit
to andrewvc/logstash-output-elasticsearch
that referenced
this pull request
May 20, 2016
Fixes logstash-plugins#377 Before acceptance this will need elastic/elasticsearch-ruby#281 Merged in and used for the ruby client We no longer need the concurrent ruby gem Support manticore pool options directly Add sniffing to info log level Proper info logging for sniffing Exponential backoff for retries Make this actually multithreaded by omitting request mutex More fixes for threadsafety Add healthcheck option Checkpoint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal here is to use a single manticore pool + a single ES ruby client + a single sniffer for future Logstashes.
The issue currently is that sniffing redefines the manticore connection pool. This patch makes the whole thing threadsafe and more efficiently use connections.