3

This is running in multiple Sidekiq instances and workers at the same time and it seems that is has generated a couple of issues, like instances getting assigned the "It was alerted recently" error when shouldn't and the opposite.

It is rare, but it is happening, is this the problem or maybe it is something else?

class BrokenModel < ActiveRecord::Base
  validates_with BrokenValidator

end

class BrokenValidator < ActiveModel::Validator
  def validate record
    @record = record

    check_alerted
  end

  private

  def check_alerted
    if AtomicGlobalAlerted.new(@record).valid?
      @record.errors[:base] << "It was alerted recently"
    end

    p "check_alerted: #{@record.errors[:base]}"
  end
end

class AtomicGlobalAlerted
  include Redis::Objects
  attr_accessor :id

  def initialize id
    @id = id
    @fredis = nil

    Sidekiq.redis do |redis|
        @fredis = FreshRedis.new(redis, freshness: 7.days, granularity: 4.hours)
    end
  end

  def valid?
    @fredis.smembers.includes?(@id)
  end
end
felipeclopes
  • 4,010
  • 2
  • 25
  • 35

2 Answers2

3

We were experiencing something similar at work and after A LOT of digging finally figured out what was happening.

The class method validates_with uses one instance of the validator (BrokenValidator) to validate all instances of the class you're trying to validate (BrokenModel). Normally this is fine but you are assigning a variable (@record) and accessing that variable in another method (check_alerted) so other threads are assigning @record while other threads are still trying to check_alerted.

There are two ways you can fix this:

1) Pass record to check_alerted:

class BrokenValidator < ActiveModel::Validator
  def validate(record)    
    check_alerted(record)
  end

  private

  def check_alerted(record)
    if AtomicGlobalAlerted.new(record).valid?
      record.errors[:base] << "It was alerted recently"
    end

    p "check_alerted: #{record.errors[:base]}"
  end
end

2) Use the instance version of validates_with which makes a new validator instance for each model instance you want to validate:

class BrokenModel < ActiveRecord::Base
  validate :instance_validators

  def instance_validators
    validates_with BrokenValidator
  end

end

Either solution should work and solve the concurrency problem. Let me know if you experience any other issues.

Jose Castellanos
  • 528
  • 4
  • 11
-1

I believe there are some thread safety issues in rails but we can overcome them by taking necessary precautions.

The local variables, such as your local var, are local to each particular invocation of the method block. If two threads are calling this block at the same time, then each call will get its own local context variable and those won't overlap unless there are shared resources involved: instance variables like (@global_var), static variables (@@static_var), globals ($global_var) can cause concurrency problems.

You are using instance variable, just instantiate it every time you are coming to the validate_record method and hopefully your problem will go away like :

def validate record
    @record.errors[:base] = []
    @record = record
    check_alerted
end

For more details you can visit this detailed link

Or try to study about rails configs here : link

Community
  • 1
  • 1
Muhammad Ali
  • 2,173
  • 15
  • 20