Merge pull request #1327 from jsynack/qf-nodehooks-desync

Quick-fix: repair some logic for @jobs_done.

1 files changed, 6 insertions, 1 deletions

diff --git a/lib/oxidized/worker.rb b/lib/oxidized/worker.rb
index 692b060..5d5fc01 100644
--- a/lib/oxidized/worker.rb
+++ b/lib/oxidized/worker.rb
@@ -42,9 +42,9 @@ module Oxidized
       node.stats.add job
       @jobs.duration job.time
       node.running = false
-      @jobs_done += 1 # needed for worker_done event
 
       if job.status == :success
+        @jobs_done += 1 # needed for :nodes_done hook
         Oxidized.Hooks.handle :node_success, :node => node,
                                              :job => job
         msg = "update #{node.name}"
@@ -66,6 +66,11 @@ module Oxidized
           msg += ", retry attempt #{node.retry}"
           @nodes.next node.name
         else
+          # Only increment the @jobs_done when we give up retries for a node (or success).
+          # As it would otherwise cause @jobs_done to be incremented with generic retries.
+          # This would cause :nodes_done hook to desync from running at the end of the nodelist and
+          # be fired when the @jobs_done > @nodes.count (could be mid-cycle on the next cycle).
+          @jobs_done += 1
           msg += ", retries exhausted, giving up"
           node.retry = 0
           Oxidized.Hooks.handle :node_fail, :node => node,