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

"worst case" error handling doesn't #35

Closed
doismellburning opened this issue Jan 6, 2015 · 2 comments
Closed

"worst case" error handling doesn't #35

doismellburning opened this issue Jan 6, 2015 · 2 comments

Comments

@doismellburning
Copy link

def flush(actions, teardown=false)
begin
@logger.debug? and @logger.debug "Sending bulk of actions to client[#{@client_idx}]: #{@host[@client_idx]}"
@current_client.bulk(actions)
rescue => e
@logger.error "Got error to send bulk of actions to elasticsearch server at #{@host[@client_idx]} : #{e.message}"
raise e
ensure
unless @protocol == "node"
@logger.debug? and @logger.debug "Shifting current elasticsearch client"
shift_client
end
end
# TODO(sissel): Handle errors. Since bulk requests could mostly succeed
# (aka partially fail), we need to figure out what documents need to be
# retried.
#
# In the worst case, a failing flush (exception) will incur a retry from Stud::Buffer.
end # def flush
says "In the worst case, a failing flush (exception) will incur a retry from Stud::Buffer."

I believe that not to be the case. After changing line 449 (for debugging reasons) to:

      tmp = @current_client.bulk(actions)
      @logger.debug tmp

my log looked like:

{:timestamp=>"2015-01-06T13:38:32.614000+0000", :message=>"Sending bulk of actions to client[0]: lorem.ipsum", :level=>:debug, :file=>"logstash/outputs/elasticsearch.rb", :line=>"443"}
{:timestamp=>"2015-01-06T13:38:32.892000+0000", :level=>:debug, "took"=>1, "errors"=>true, "items"=>[{"index"=>{"_index"=>"logstash-2015.01.01", "_type"=>"supervisor", "_id"=>nil, "status"=>404, "error"=>"IndexMissingException[[logstash-2015.01.01] missing]"}}, {"index"=>{"_index"=>"logstash-2015.01.01", "_type"=>"supervisor", "_id"=>nil, "status"=>404, "error"=>"IndexMissingException[[logstash-2015.01.01] missing]"}}], :file=>"logstash/outputs/elasticsearch.rb", :line=>"445"}
{:timestamp=>"2015-01-06T13:38:32.893000+0000", :message=>"Shifting current elasticsearch client", :level=>:debug, :file=>"logstash/outputs/elasticsearch.rb", :line=>"451"}

Note the lack of "Got error to send bulk of actions to elasticsearch server" - from a user perspective, until I made my source modification, the entirety of my writes were erroring silently.

(This was with the version from LS 1.5.0-beta1)

(It is possible my definition of "partially fail" / "failing flush" differs from that of $author - I guess the HTTP connection was absolutely fine...)

@talevy
Copy link
Contributor

talevy commented Jan 6, 2015

Yes, We apologize for this.
If any action within the bulk request fails, it is dropped :(
We should be pushing out the fix for this anytime now.
If you are interested in following, the PR is here: #2

In general, the update will apply the following behavior:

If an action is unsuccessful, it will be retried in the background. If it continues to fail after a certain amount of tries, it is printed out in a log message and dropped (this is the best we can do for now without extra persistence). If you have any thoughts or concerns about this policy, please let us know!

@suyograo
Copy link
Contributor

@doismellburning #2 should fix this. please let us know if its not the case

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

No branches or pull requests

3 participants