FIX: in redis readonly raise an exception from DistributedMutex

If we detect redis is in readonly we can not correctly get a mutex
raise an exception to notify caller

When getting optimized images avoid the distributed mutex unless
for some reason it is the first call and we need to generate a thumb

In redis readonly no thumbnails will be generated

app/models/optimized_image.rb

@@ -38,18 +38,23 @@ class OptimizedImage < ActiveRecord::Base
       end
     end
 
+    # prefer to look up the thumbnail without grabbing any locks
+    thumbnail = find_by(upload_id: upload.id, width: width, height: height)
+
+    # correct bad thumbnail if needed
+    if thumbnail && thumbnail.url.blank?
+      thumbnail.destroy
+      thumbnail = nil
+    end
+
+    return thumbnail if thumbnail
+
     lock(upload.id, width, height) do
-      # do we already have that thumbnail?
+      # may have been generated since we got the lock
       thumbnail = find_by(upload_id: upload.id, width: width, height: height)
 
-      # make sure we have an url
-      if thumbnail && thumbnail.url.blank?
-        thumbnail.destroy
-        thumbnail = nil
-      end
-
       # return the previous thumbnail if any
-      return thumbnail unless thumbnail.nil?
+      return thumbnail if thumbnail
 
       # create the thumbnail otherwise
       original_path = Discourse.store.path_for(upload)

lib/distributed_mutex.rb

@@ -7,15 +7,26 @@ class DistributedMutex
 
   def initialize(key, redis = nil)
     @key = key
+    @using_global_redis = true if !redis
     @redis = redis || $redis
     @mutex = Mutex.new
   end
 
+  CHECK_READONLY_ATTEMPT ||= 10
+
   # NOTE wrapped in mutex to maintain its semantics
   def synchronize
+
     @mutex.lock
+    attempts = 0
+
     while !try_to_get_lock
       sleep 0.001
+      attempts += 1
+      # in readonly we will never be able to get a lock
+      if @using_global_redis && attempts > CHECK_READONLY_ATTEMPT
+        raise Discourse::ReadOnly
+      end
     end
 
     yield

spec/components/distributed_mutex_spec.rb

@@ -45,4 +45,29 @@ describe DistributedMutex do
     }.to raise_error(ThreadError)
   end
 
+  context "readonly redis" do
+    before do
+      $redis.slaveof "127.0.0.1", "99991"
+    end
+
+    after do
+      $redis.slaveof "no", "one"
+    end
+
+    it "works even if redis is in readonly" do
+      m = DistributedMutex.new("test_readonly")
+      start = Time.now
+      done = false
+
+      expect {
+        m.synchronize do
+          done = true
+        end
+      }.to raise_error(Discourse::ReadOnly)
+
+      expect(done).to eq(false)
+      expect(Time.now - start).to be < (1.second)
+    end
+  end
+
 end