Merge pull request #290 from airbnb/ramya/throttle-zk-discover

Throttle discover in zookeeper service watcher

Gemfile.lock

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

lib/synapse/service_watcher/zookeeper.rb

@@ -272,7 +272,27 @@ def watch
     # handles the event that a watched path has changed in zookeeper
     def watcher_callback
       @callback ||= Proc.new do |event|
-        # Set new watcher
+        # We instantiate ZK client with :thread => :per_callback
+        # https://github.com/zk-ruby/zk/wiki/EventDeliveryModel#thread-per-callback
+        # Only one thread will be executing the callback at a time
+        #
+        # 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.
+        # 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'] > 0 && @discovery['discovery_jitter'] < 120
+            log.info "synapse: sleeping for discovery_jitter=#{@discovery['discovery_jitter']} seconds for service:#{@name}"
+            sleep @discovery['discovery_jitter']
+          else
+            log.warn "synapse: invalid discovery_jitter=#{@discovery['discovery_jitter']} for service:#{@name}"
+          end
+        end
         watch
         # Rediscover
         discover

lib/synapse/version.rb

@@ -1,3 +1,3 @@
 module Synapse
-  VERSION = "0.16.7"
+  VERSION = "0.16.8"
 end

spec/lib/synapse/service_watcher_zookeeper_spec.rb

@@ -172,6 +172,15 @@
       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 "generator_config_path" do
       let(:discovery) { { 'method' => 'zookeeper', 'hosts' => 'somehost', 'path' => 'some/path', 'generator_config_path' => generator_config_path } }
       before :each do