Merge pull request #293 from airbnb/ramya/backoff-jitter

Avoid discovery_jitter sleep on first event

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    synapse (0.16.10)
+    synapse (0.16.11)
       aws-sdk (~> 1.39)
       docker-api (~> 1.7)
       dogstatsd-ruby (~> 3.3.0)

lib/synapse/service_watcher/zookeeper.rb

@@ -280,17 +280,21 @@ def watcher_callback
         #
         # We call watch on every callback, but do not call zk.register => change callback, except the first time.
         #
-        # We sleep. We loose / do not get any events during this time for this service.
+        # We sleep if we have not slept in discovery_jitter seconds
+        # We loose / do not get any events during this time for this service.
         # This helps with throttling discover.
         #
         # We call watch and discover on every callback.
         # We call exists(watch), children(discover) and get(discover) with (:watch=>true)
         # This re-initializes callbacks just before get scan. So we do not miss any updates by sleeping.
         #
-        if @watcher && @discovery['discovery_jitter']
+        if @discovery['discovery_jitter']
           if @discovery['discovery_jitter'].between?(MIN_JITTER, MAX_JITTER)
-            log.info "synapse: sleeping for discovery_jitter=#{@discovery['discovery_jitter']} seconds for service:#{@name}"
-            sleep @discovery['discovery_jitter']
+            if @last_discovery && (Time.now - @last_discovery) < @discovery['discovery_jitter']
+              log.info "synapse: sleeping for discovery_jitter=#{@discovery['discovery_jitter']} seconds for service:#{@name}"
+              sleep @discovery['discovery_jitter']
+            end
+            @last_discovery = Time.now
           else
             log.warn "synapse: invalid discovery_jitter=#{@discovery['discovery_jitter']} for service:#{@name}"
           end

lib/synapse/version.rb

@@ -1,3 +1,3 @@
 module Synapse
-  VERSION = "0.16.10"
+  VERSION = "0.16.11"
 end

spec/lib/synapse/service_watcher_zookeeper_spec.rb

@@ -172,22 +172,38 @@
       expect(subject.ping?).to be false
     end
 
-    it 'throttles discovery if discovery_jitter is set' do
-      discovery['discovery_jitter'] = 25
-      expect(subject).to receive(:watch)
-      expect(subject).to receive(:discover)
-      expect(subject).to receive(:sleep).with(25)
-      subject.instance_variable_set(:@watcher, instance_double(ZK::EventHandlerSubscription))
-      subject.send(:watcher_callback).call
-    end
+    context 'watcher_callback' do
+      let (:time_now) { Time.now }
+      before :each do
+        expect(subject).to receive(:watch)
+        expect(subject).to receive(:discover)
+        subject.instance_variable_set(:@watcher, instance_double(ZK::EventHandlerSubscription))
+        discovery['discovery_jitter'] = 25
+        allow(Time).to receive(:now).and_return(time_now)
+      end
 
-    it 'do not throttle on invalid discovery_jitter' do
-      discovery['discovery_jitter'] = Synapse::ServiceWatcher::ZookeeperWatcher::MAX_JITTER + 1
-      expect(subject).to receive(:watch)
-      expect(subject).to receive(:discover)
-      expect(subject).not_to receive(:sleep)
-      subject.instance_variable_set(:@watcher, instance_double(ZK::EventHandlerSubscription))
-      subject.send(:watcher_callback).call
+      it 'does not sleep if there was no discovery in last discovery_jitter seconds' do
+        subject.instance_variable_set(:@last_discovery, time_now - (discovery['discovery_jitter'] + 1))
+        expect(subject).not_to receive(:sleep)
+        subject.send(:watcher_callback).call
+      end
+
+      it 'does not sleep if @last_discovery is not set - first time' do
+        expect(subject).not_to receive(:sleep)
+        subject.send(:watcher_callback).call
+      end
+
+      it 'sleep if there was a discovery in last discovery_jitter seconds' do
+        subject.instance_variable_set(:@last_discovery, time_now - (discovery['discovery_jitter'] - 1))
+        expect(subject).to receive(:sleep).with(discovery['discovery_jitter'])
+        subject.send(:watcher_callback).call
+      end
+
+      it 'do not throttle on invalid discovery_jitter' do
+        discovery['discovery_jitter'] = Synapse::ServiceWatcher::ZookeeperWatcher::MAX_JITTER + 1
+        expect(subject).not_to receive(:sleep)
+        subject.send(:watcher_callback).call
+      end
     end
 
     context "generator_config_path" do