basho / riak_cs

Riak CS is simple, available cloud storage built on Riak.
http://docs.basho.com/riakcs/latest/
Apache License 2.0
566 stars 95 forks source link

Retry or backpressure in overload errors in GETs #846

Open alexmoore opened 10 years ago

alexmoore commented 10 years ago

Related to Zendesk ticket #7747

While Riak CS is in the overload state, an object get can return a 200 OK and no content. Sometimes retries will be able to fetch the object.

We should probably return a {500, server_overload} or something of the sort instead.

All the crash.log entries start with:

  crasher:
    initial call: mochiweb_acceptor:init/3
    pid: <0.6062.65>
    registered_name: []
    exception exit: {{normal,{gen_fsm,sync_send_event,[<0.8256.89>,get_next_chunk,infinity]}},[{gen_fsm,sync_send_event,3,[{file,"gen_fsm.erl"},{line,214}]},{riak_cs_wm_utils,streaming_get,4,[{file,"src/riak_cs_wm_utils.erl"},{line,272}]},{webmachine_decision_core,'-make_encoder_stream/3-fun-0-',3,[{file,"src/webmachine_decision_core.erl"},{line,667}]},{webmachine_request,send_stream_body_no_chunk,2,[{file,"src/webmachine_request.erl"},{line,334}]},{webmachine_request,send_response,3,[{file,"src/webmachine_request.erl"},{line,398}]},{webmachine_request,call,2,[{file,"src/webmachine_request.erl"},{line,251}]},{webmachine_decision_core,wrcall,1,[{file,"src/webmachine_decision_core.erl"},{line,42}]},{webmachine_decision_core,finish_response,3,[{file,"src/webmachine_decision_core.erl"},{line,92}]}]}
    ancestors: [object_web_mochiweb,riak_cs_sup,<0.141.0>]

Will email more details to internal list.

alexmoore commented 10 years ago

Reviewed the console.log again, some more information to add around the files I sent internally. Seems we always see the pattern of <<"overload">>, then "cannot get chunk", then CRASH REPORT.

2014-05-01 13:55:45.978 [error] <0.8495.89>@riak_cs_block_server:try_local_get:288 do_get_block: other error 1: <<"overload">>
2014-05-01 13:55:45.978 [error] <0.8479.89>@riak_cs_get_fsm:waiting_chunks:311 riak_cs_get_fsm: Cannot get S3 <<"cloud">> <<"members/xxx/00000/loadtest/blah.jpg">> block# {<<95,188,169,210,126,245,68,164,129,95,17,85,146,175,48,71>>,0}: {error,notfound}
2014-05-01 13:55:45.978 [error] <0.5916.63> CRASH REPORT Process <0.5916.63> with 0 neighbours exited with reason: {normal,{gen_fsm,sync_send_event,[<0.8479.89>,get_next_chunk,infinity]}} in gen_fsm:sync_send_event/3 line 214
dmitrizagidulin commented 10 years ago

Also encountered in Zendesk ticket https://basho.zendesk.com/agent/#/tickets/7862

shino commented 10 years ago

It is possible to add retry logic to riak_cs_block_server... It's better, I think, that HTTP clients retry (end-to-end) because riak cs can handle range get (like wget does when response is terminated prematurally).

I will close this, but if there are any idea to handle the situation better, please feel free to reopen.

reiddraper commented 10 years ago

+1 to having the Riak CS clients simply retry. We should be propagating the overloaded backpressure back to users. That being said, we could certainly stand to have better error-messages in Riak CS when this happens.

alexmoore commented 10 years ago

+1 to better error messages.

shino commented 10 years ago

Reopen for better error messages.

kuenishi commented 10 years ago

+1 to both retry backpressure and that seems what S3 is doing (it sometimes gets very slow).

kuenishi commented 10 years ago

This may be solved via #929, which was not overload, but just caused by timeout in 5 seconds without any retry - fixed to retry with interval and longer timeout.

shino commented 10 years ago

Memo for self:

Re-read the code after #929 review, this seems sufficient to add Why =:= <<"overload">> at https://github.com/basho/riak_cs/blob/release/1.5/src/riak_cs_block_server.erl#L285 and go down to handle_local_notfound (which handles other errors than notfound :sweat_smile: ).

Alternate choice is adding <<"overload">> to the case branch just above and retry from n_val=1. This may be better choice if overload is cluster-wide (less load to the cluster). Otherwise (overload is only for the node), default n_val seems good.