{"action":"created","issue":{"url":"https://api.github.com/repos/mperham/sidekiq/issues/3470","repository_url":"https://api.github.com/repos/mperham/sidekiq","labels_url":"https://api.github.com/repos/mperham/sidekiq/issues/3470/labels{/name}","comments_url":"https://api.github.com/repos/mperham/sidekiq/issues/3470/comments","events_url":"https://api.github.com/repos/mperham/sidekiq/issues/3470/events","html_url":"https://github.com/mperham/sidekiq/issues/3470","id":227577981,"number":3470,"title":"Solution for Duplicate JIDs processed by different Threads","user":{"login":"d11wtq","id":37948,"avatar_url":"https://avatars1.githubusercontent.com/u/37948?v=3","gravatar_id":"","url":"https://api.github.com/users/d11wtq","html_url":"https://github.com/d11wtq","followers_url":"https://api.github.com/users/d11wtq/followers","following_url":"https://api.github.com/users/d11wtq/following{/other_user}","gists_url":"https://api.github.com/users/d11wtq/gists{/gist_id}","starred_url":"https://api.github.com/users/d11wtq/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/d11wtq/subscriptions","organizations_url":"https://api.github.com/users/d11wtq/orgs","repos_url":"https://api.github.com/users/d11wtq/repos","events_url":"https://api.github.com/users/d11wtq/events{/privacy}","received_events_url":"https://api.github.com/users/d11wtq/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2017-05-10T06:22:00Z","updated_at":"2017-05-10T22:51:13Z","closed_at":"2017-05-10T14:15:50Z","body":"Ruby version: 2.4.1 and 2.2.2\r\nSidekiq / Pro / Enterprise version(s): 4.2.10\r\n\r\nIn relation to #3303, we have written the following middleware to compare JIDs with Thread IDs using Redis and avoid processing the job if there is a mismatch. Does this look like it could cause bigger issues for us, or does what we're doing here seem sensible? The duplicates we saw in our logs (hundreds of thousands of them over a few days) were all across different TIDs (in fact, possibly coincidentally, different PIDs).\r\n\r\nThe actual issue was in relation to sending push notifications, so the issue was considered quite serious for us. Many customers got 3 duplicate notifications due to duplication of Sidekiq JIDs via a series of Sidekiq jobs that enqueue other Sidekiq jobs.\r\n\r\nFWIW, the code application had worked beautifully for 3 months until 3 days ago when this issue started and continued until today.\r\n\r\nThe duplication of JIDs doesn't appear to have been linked to timeouts in our case. Something else was going on, possibly with our Redis sentinels.\r\n\r\n``` ruby\r\nmodule Core\r\n  # Protect against processing the same JID twice in Sidekiq.\r\n  #\r\n  # We observed a serious defect in production, in which Sidekiq jobs were\r\n  # being enqueued once in the code, but processed twice in the worker. We were\r\n  # able to definitively see the same JIDs repeated across different TIDs\r\n  # within the log files. Not once or twice, but 75% of the time. JIDs should\r\n  # be unique, however it appears that in the case of Redis issues, the Redis\r\n  # client will retry operations, leading to these potential duplicate jobs.\r\n  #\r\n  # This middleware compares the ID of the current thread with the ID stored in\r\n  # a Redis-based lock (using SETNX) and refuses to process the job if the IDs\r\n  # do not match.\r\n  class SidekiqDuplicateJobMiddleware\r\n    def initialize(ttl:)\r\n      @ttl = ttl\r\n    end\r\n\r\n    # This is a Sidekiq Middleware method.\r\n    #\r\n    # Invokes job only if its JID is not yet processed by another worker.\r\n    #\r\n    # @param [Sidekiq::Worker] worker\r\n    #   the sidekiq worker that will process the job\r\n    # @param [Hash] job\r\n    #   the job payload\r\n    # @param [String] queue\r\n    #   the name of the queue the job is processed on\r\n    def call(worker, job, queue)\r\n      yield unless processed?(job['jid'])\r\n    end\r\n\r\n    private\r\n\r\n    def tid\r\n      # https://github.com/mperham/sidekiq/blob/d8f11c26518dbe967880f76fd23bb99e9d2411d5/lib/sidekiq/logging.rb#L14\r\n      Thread.current.object_id.to_s(36)\r\n    end\r\n\r\n    def acquired_job_key(jid)\r\n      format('acquired_job:%s', jid)\r\n    end\r\n\r\n    def acquired_by_tid(jid)\r\n      Sidekiq.redis do |redis|\r\n        redis.setnx(acquired_job_key(jid), tid)\r\n        redis.expire(acquired_job_key(jid), @ttl)\r\n        redis.get(acquired_job_key(jid))\r\n      end\r\n    end\r\n\r\n    def processed?(jid)\r\n      acquired_by_tid(jid) != tid\r\n    end\r\n  end\r\nend\r\n```"},"comment":{"url":"https://api.github.com/repos/mperham/sidekiq/issues/comments/300634783","html_url":"https://github.com/mperham/sidekiq/issues/3470#issuecomment-300634783","issue_url":"https://api.github.com/repos/mperham/sidekiq/issues/3470","id":300634783,"user":{"login":"mperham","id":2911,"avatar_url":"https://avatars1.githubusercontent.com/u/2911?v=3","gravatar_id":"","url":"https://api.github.com/users/mperham","html_url":"https://github.com/mperham","followers_url":"https://api.github.com/users/mperham/followers","following_url":"https://api.github.com/users/mperham/following{/other_user}","gists_url":"https://api.github.com/users/mperham/gists{/gist_id}","starred_url":"https://api.github.com/users/mperham/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mperham/subscriptions","organizations_url":"https://api.github.com/users/mperham/orgs","repos_url":"https://api.github.com/users/mperham/repos","events_url":"https://api.github.com/users/mperham/events{/privacy}","received_events_url":"https://api.github.com/users/mperham/received_events","type":"User","site_admin":false},"created_at":"2017-05-10T22:51:13Z","updated_at":"2017-05-10T22:51:13Z","body":"You're welcome.  I have no idea what your business is or its tolerance to failure but if your \"safe\" sentinel option is less reliable than Redis by itself, perhaps a rethink is in order.  I would personally either run Redis standalone myself or pay for Redis Cluster as a SaaS (Elasticache, Redis Labs, etc)."}}
{"actor":{"display_login":"mperham"}}