Add a fault_tolerant flag to fail-over to stale cache. #91

Open
wants to merge 43 commits into
from

Conversation

Projects
None yet
9 participants

Rack::Cache includes an option to enable fault tolerant caching, so that stale cached results can be returned if the downstream service is unavailable. Just set :fault_tolerant => true in the options hash.

All automated tests continue to pass.

I would bracket the end as well if you feel it is important to reference this.

I left it in by accident, it was just to help me find my new test while I was working on it.

Do we really want to add this dependency? Seems to me that if you are modifying errors that can be caught, it would be better to just add some kind of configuration of that, if it can be done.

Yeah, we can't assume this'll be used in a faraday context as this gem can be used for rack-middleware as well as faraday middleware.

Do you need to include the zero. If headers['Age'] is nil that will give a zero and in the logic before, if there is a 'Age' header, it wins regardless and presumbably the 0 corresponses to nil.

Also, you don't need max.to_i since everything will already be an integer.

The 'Age' header will generally be zero (assuming no intermediate caches), so it usually won't win. The zero is for the case where our client computer's time is not in sync to the server's, and we get a date header from the server that is in the future. It seemed like a bad idea to have a negative age.

Suggested: whitespace +=

Indentation here and below

clabrunda and others added some commits Apr 18, 2013

Merge pull request #3 from mdsol/fix_etag_bug
Don't set HTTP_IF_NONE_MATCH to nil if etags missing
Chaim Solomon
Don't assume that we have middleware_options.
For Eureka-client we can assume them to be present but in other contexts they may not be around.
Chaim Solomon
Added retry-logic: Retry <retry> number of times, then (if fault-tole…
…rant) fail-over to stale date or just fail.
lib/rack/cache/context.rb
@@ -15,6 +15,10 @@ class Context
# The Rack application object immediately downstream.
attr_reader :backend
+ # Set of exceptions that indicate a network connection failure.
+ # TODO: Make these configurable, to work in a non-faraday environment.
+ EXCEPTION_CLASSES = Set.new %w(Timeout::Error Faraday::Error::ConnectionFailed Faraday::Error::TimeoutError)
@rtomayko

rtomayko Oct 10, 2013

Owner

Hmm, maybe just move these to a class instance variable at Rack::Cache.network_failure_exceptions for now so it's a bit more configurable?

lib/rack/cache/context.rb
+ entry.headers['Age'] = age
+ record "Fail-over to stale cache data with age #{age} due to #{e.class.name}: #{e.to_s}"
+ entry
+ end
@rtomayko

rtomayko Oct 10, 2013

Owner

Hmm I guess it'd be tricky with the retry var but it'd be great to pull this out into a separate method. It'd be a bit more descriptive, could possibly get rid of a begin/end, etc.

Owner

rtomayko commented Oct 10, 2013

This is really cool. Thanks for the patch!

See what you think about the comments I left and I'll circle back here to see about merging.

fosdev commented Oct 11, 2013

@rtomayko Comments addressed. Feel free to re-review.

@swrobel swrobel referenced this pull request in redis-store/redis-store Oct 29, 2013

Closed

Redis Failure #175

mszenher commented Jul 8, 2014

@rtomayko Any chance this can be merged? Thanks.

roberto commented Aug 29, 2014

Is there any progress here?

tulios commented Sep 19, 2014

It would be nice to have this merged, any progress?

I think this is just awaiting @rtomayko 's final approval and merge. @rtomayko, is there anything more you want us contributors to do in this PR or is it ready to merge? Thanks.

Collaborator

grosser commented Oct 8, 2015

could we pass in a validator lambda/object instead so that users can do whatever logic they want ?

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