DEV Community: Kyle d'Oliveira

More feedback! Quantity becomes quality

Kyle d'Oliveira — Wed, 15 Nov 2023 17:27:23 +0000

In David Bayles and Ted Orland's book, Art & Fear, there's a captivating story that has always stuck with me. This is a story that highlights a timeless argument: quantity versus quality. It goes like this:

"[A] ceramics teacher announced on opening day that he was dividing the class into two groups. All those on the left side of the studio would be graded solely on the quantity of work they produced, while those on the right would be graded solely on its quality...

Well, came grading time and a curious fact emerged: the works of highest quality were all produced by the group being graded for quantity. It seems that while the 'quantity' group was busily churning out piles of work — and learning from their mistakes — the 'quality' group had sat theorizing about perfection, and in the end had little more to show for their efforts than grandiose theories and a pile of dead clay."

This story isn't just about art. It's a lesson that extends to the realm of engineering, where quality and quantity are often perceived as opposing forces. Initially in my career, I thought achieving high-quality results required a significant investment of time. However, over time I discovered that the ceramics teacher's story can be applied to software engineering — a field that, much like art, is subjective and reliant on experience to distinguish "good code" from the rest.

Good code is more than just functional — it's maintainable, extensible, secure, and reliable. The best way to learn how to write such code is by either experiencing the consequences of poor code quality or learning from the experiences of others who have already learned those lessons.

In a previous role as a developer manager overseeing over 25 engineers, I witnessed a pattern that further reinforced the notion that quality and quantity are not necessarily at odds. Some engineers focused on perfection, aiming for flawless code. It's often said that "perfect is the enemy of good," and indeed, those engineers produced fewer lines of code but strived to maintain a high quality standard. On the flip side, some engineers were obsessed with producing and reviewing as much code as possible, quickly working through task lists. Those "quantity" engineers initially faced more bugs and required additional oversight, especially while getting started in their careers.

At first glance, it seemed like a classic case of quality versus quantity. However, what sets the two groups apart is the volume of feedback. What the ceramics teacher discovered in the story is the same story I saw with these engineers. The "quantity" group received abundant feedback because they produced and reviewed more work, ultimately improving their code quality over time much faster than the "quality" group.

It's not a battle of quality against quantity. Effectively, in software engineering, the more feedback you can receive, either through self-reflection or from others, the more you can learn and improve the quality of your work. There are also many ways to generate feedback. Here are some suggestions:

Do code reviews

Most people are used to addressing feedback on their own code. But when someone makes a comment, it is worth thinking about why the reviewer made that comment. It could be that the reviewer is leaning into some experience they can share. However, it is also important to get feedback on your ability to review code as well. Having two reviewers is an amazing tactic most teams can do. Not only does this provide additional feedback for the code writer, but it also gives an opportunity for the reviewers to get feedback from each other. Someone unfamiliar with an area of code might miss something, but when they can see someone else comment on it, it becomes an opportunity for both the reviewer and the writer to learn something.

Monitor deployed code

When your code goes out, how do you know it is working? And working well? It is always worth monitoring your features and looking for bugs and performance regressions, or just looking at adoption metrics. What you notice while monitoring can help you, the author, understand what slipped through so you can learn and improve. You can then share these learnings with your team and your code reviewers. Thinking about how you will observe whether things are working correctly or not ahead of time can also have a big impact on the quality of the code you write.

Debug unfamiliar areas of the code

In my experience, it is common for people unfamiliar with certain areas of the code to do what they can to avoid them altogether. Unfortunately, the end result of that tactic is the code area will stay unknown. Bugs, either discovered by your customers or error tracking software, can help guide you in where to explore. By jumping into unfamiliar areas of code, even if you do not "solve" the bug, you can learn new areas of the code, tricks for getting up to speed quickly, and debugging techniques. You might even identify where developer tooling is lacking. By stepping into an area that is a little outside of your comfort zone, your comfort zone will expand.

Create more pull requests

Pull requests don't need to happen at the end once all development is completed on a feature. Get your work in front of people fast. It is much easier to pivot early on than in the later stages. You can break a feature into small sections and ask for feedback on each piece, or get some eyes on your code early with a demo or code walkthrough. To go with the previous point, tackling bugs in unfamiliar areas of code is another way to start getting incremental feedback. And if you haven't gotten feedback on your work in a few days, then you should try to go gather some soon.

Keep in mind — the more complex a pull request is, the harder it is to review. The reviewer will need to maintain a lot of context which can lead to a shallow depth of the review. Or, it will take a long time to complete. The easier you make the pull request to review, the better feedback you are going to receive the easier it will be to find bugs.

Grouping pull requests on a single type of change is one way you can break large pull requests apart. One way to do this might be to have a single pull request that refactors a complex piece of code without modifying or changing the existing behavior, then a follow-up pull request with just the new changes. Another way might be to write up a pull request that is just a model and a database migration, then another for the controller and unit tests, followed by another with the UI and integration tests. Remember that pull requests can be stacked on top of each other. And just because it is a self contained piece doesn't mean that it is going to be deployed.

Share your mistakes

This isn't about blame or keeping a record of who messed up. By sharing your mistakes, your team might be able to identify and avoid those same mistakes too. Explaining what went wrong or what things you missed also helps to solidify your internal learning. Based on the lesson, you could share it at various levels — with the people who reviewed your code, your team, or the entire department.

Aiming for feedback quantity instead of quality might be a big shift. But in reality, you are still focusing on quality.

The goal is to use feedback, whether you give or receive it, to vastly increase your skills and the pace of your learning. Regardless of what you are working on, think about how and when you can get feedback on it. It is worth discussing with your team all of the various ways you can do this.

Engineering is a team effort that requires cooperation from everyone. But you can lead by example here. If you give solid feedback to others, you can supercharge your team's focus on quality. This can become a virtuous cycle where a supercharged teammate will provide great feedback to you or others in return. Everyone wins and everyone gets better.

At the end of the day, quality and quantity are desirable goals and they are both achievable. Churn out lots of code, learn from mistakes and the experiences of others, and you'll have high quality code too. That way, you can all have much more to show than grandiose theories and a pile of dead code.

Off to the races: 3 ways to avoid race conditions

Kyle d'Oliveira — Tue, 05 Sep 2023 19:23:29 +0000

What is a race condition?

I searched for a good definition of a race condition and this is the best I found:

A race condition is unanticipated behavior caused by multiple processes interacting with shared resources in a different order than expected.

This is quite the mouthful and it is still not very clear how race conditions show up in Rails.

Using Rails, we are always working with multiple processes — each request or background job is an individual process that can operate mostly independent of other processes.

We are also always working with shared resources. Does the application use a relational database? That's a shared resource. Does the application use some kind of caching server? Yup, that's a shared resource. Do you use some kind of external API? You guessed it — that's a shared resource.

There are two example categories of race conditions that I would like to talk about and then touch on how to approach addressing them.

Read-modify-write

The read-modify-write category is a type of race condition where one process will read values from a shared resource, modify the value within memory, and then attempt to write it back to the shared resource. This seems very straightforward when we look at it through the lens of a single process. But when a second process comes up, it can result in some unanticipated behavior.

Consider code that looks like this:

class IdeasController < ActionController::Base

  def vote
    @idea = Idea.find(params[:id])
    @idea.votes += 1
    @idea.save!
  end

end

Here we are reading (Idea.find(params[:id])), modifying (@idea.votes += 1), then writing (@idea.save!).

We can see that this would increment the number of votes on an idea by one. If there was an idea with zero votes, it would end with having one vote. However, if a second request came in and read the idea from the database while it still had zero votes and incremented that value in memory, we could have a situation where two votes come in simultaneously — yet the end result is that the number of votes in the database is only one.

This is also referred to as the lost update race condition.

Check-then-act

The check-then-act category is a type of race condition where data is loaded from a shared resource, and depending on the value present, we determine if an action needs to be performed.

One of the classic examples of how this shows up is in the validates_uniqueness_of validation in Rails, like this:

class User < ActiveRecord::Base
  validates_uniqueness_of :email
end

Consider code that looks like this:

User.create(email: "demo@example.com")

With the validation in place, Rails will check if there is any existing user with that email. If there is no other, it will act by persisting the user into the database. However, what would happen if a second request was executing the same code at the same time? We could end up in a situation where both requests check to determine if there is duplicate data (and there is none) — then they will both act by saving the data, resulting in a duplicate user in the database.

Addressing race conditions

There is no silver bullet for fixing race conditions, but there are a handful of strategies that can be leveraged for any particular problem. There are three main categories for removing race conditions:

1. Remove the critical section

While this could be viewed as deleting the offending code, sometimes you can refactor the code so that it isn't vulnerable to race conditions. Other times, you can look into atomic operations.

An atomic operation is one where no other process can interrupt the operation so you know it will always execute as a single unit.

For the read-modify-write example, instead of incrementing the idea votes in memory, they could be incremented in the database:

@ideas.increment!(:votes)

That will execute sql that looks like this:

UPDATE "ideas" 
  SET "votes" = COALESCE("votes", 0) + 1 
  WHERE "ideas"."id" = 123

Utilizing this would not be subject to the same race conditions.

For the check-then-act example, instead of allowing Rails to validate the model, we could insert the record directly into the database with an upsert:

User.where(email: "demo@example.com")
  .upsert({}, unique_by: :email)

That will insert the record into the database. If there is a conflict on email (which would require a unique index on email) it will simply ignore the insert.

2. Detect and recover

Sometimes you cannot remove the critical section. It is possible there may be an atomic action, but it doesn't quite work in a way that the code requires. In those situations, you can try a detect and recover approach. With this approach, safeguards are set up that will inform you if a race condition happened. You can either gracefully
abort or retry the operation.

For the read-modify-write example, this could be done with optimistic locking. Optimistic locking is built into Rails and can allow detection of when multiple processes are operating on the same record at the same time. To enable optimistic locking, you only need to add a lock_version column to your table and Rails will automatically enable it.

change_table :ideas do |t|
  t.integer :lock_version, default: 0
end

Then when you attempt to update a record, Rails will only update it if the lock_version is the same version it was in memory. If it isn't, it will raise a ActiveRecord::StaleObjectError exception, which can be rescued to handle it. Handling it could be a retry or it could just be an error message reported back to the user.

def vote
  @idea = Idea.find(params[:id])
  @idea.votes += 1
  @idea.save!
rescue ActiveRecord::StaleObjectError
  retry
end

For the check-then-act example, this could be done with a unique index on the column, then rescuing the exception when persisting the data.

add_index :users, [:email], unique: true

With a unique index in place, if data already exists in the database with that email, Rails will raise an ActiveRecord::RecordNotUnique error and that can be rescued
and handled appropriately.

begin
  user = User.create(email: "demo@example.com")
rescue ActiveRecord::RecordNotUnique
  user = User.find_by(email: "demo@example.com")
end

Idempotency

In order to retry actions, it is important that the entire operation is idempotent. This means that if an operation is performed multiple times, the result is the
same as if it was only applied once.

For instance, imagine if a job sent out an email and it was performed whenever an idea's votes were changed. It would be really bad if an email was sent out for each retry. To make the operation idempotent, you could hold off sending the email until the entire voting operation was complete. Alternatively, you could update the implementation of the process that sends the email to only send the email if
votes changed from the last time it was sent out. If a race condition occurs and you need to retry, the first attempt at sending an email might result in a no-op and it is safe to trigger it again.

Many operations might not be idempotent — such as enqueueing a background job, sending an email, or calling a third party API.

3. Protect the code

If you cannot detect and recover, you can try to protect the code. The goal here is to create a contract where only one process can access the shared resource at a time. Effectively, you are removing concurrency — since only one process can have access to a shared resource, we can avoid most race conditions. The tradeoff though is that the more concurrency is removed, the slower the application can be as other process will wait until they are allowed access.

This could be handled using pessimistic locking that is built in with Rails. To use pessimistic locking, you can add lock to queries that are being built, and Rails will tell the database to hold a row lock on those records. The database will then prevent any other process from obtaining the lock until it is done. Be sure to wrap the code in a transaction so the database knows when to release the lock.

Idea.transaction do
  @idea = Idea.lock.find(params[:id])
  @idea.votes += 1
  @idea.save!
end

If row-level locking isn't possible, there are other tools such as Redlock or with_advisory_lock that
could be used. These will allow locking an arbitrary block of code. Using this could be as simple as something like this:

email = "demo@example.com"
User.with_advisory_lock("user_uniqueness_#{email}"} do
  User.find_or_create_by(email: email)
end

These strategies will cause processes to wait until a lock is obtained. So, they will also want to have some form of timeout to prevent a process from waiting forever — as well as some handling for what to do in the event of a timeout.

While there is no panacea for fixing race conditions, many race conditions can be fixed through these strategies. However, each problem is a little different, so
the details of the solutions can vary. You can take a look at my talk from RailsConf 2023 that goes more into detail about race conditions.

Writing and testing a custom RuboCop cop

Kyle d'Oliveira — Fri, 21 Oct 2022 17:36:44 +0000

Solving a problem is great — but keeping it from coming back is even better. As we resolve issues in our code base, we often consider how to keep that classification of issue out of the code base entirely. Sometimes we reach for RuboCop to help us police certain patterns. This also helps to document the originating issue and educates teammates on why these patterns are undesirable.

RuboCop is more than just a linter. It is highly extensible and allows you to write custom cops to enforce specific behavior. These cops can be used to create better code practices, prevent bad patterns from sneaking into a legacy code base, and provide training for other engineers. But it can be tricky to know how to create a new cop and if it will work long-term.

We can write unit tests to ensure the success of our custom cops, just as we would with any application code.
Let's explore this with an example to show how testing could be done.

Testing custom cops

With the Aha! engineering team, every model has an account_id attribute present and for security reasons, we never want this to be set via mass-assignment. To avoid this, we want to prevent certain attributes from being added to attr_accessible.

# bad
class Foo
  attr_accessible :name, :account_id
end
Foo.create(account_id: 1, name: "foo")
# good
class Foo
  attr_accessible :name
end
foo = Foo.new(name: "foo")
foo.account_id = 1
foo.save

We have a custom cop that analyzes the arguments to that method and will error if any protected attribute is present. The custom cop we have ends up looking something like this:

class RuboCop::Cop::ProtectedAttrAccessibleFields < RuboCop::Cop::Cop
  # We can define a list of attributes we want to protect
  PROTECTED_ATTRIBUTES = [
    :account_id,
  ].freeze
  # We can define an error message that is displayed when an offense is detected.
  # This can be helpful to communicate information back to other engineers
  ERROR_MESSAGE = <<~ERROR.freeze
    Only permit attributes that are safe to be completely user controlled. Typically any *_id field could be problematic.
    Instead perform direct assignment of the field after doing a scoped lookup. This is the safest way to handle user input.
    Some fields such as #{PROTECTED_ATTRIBUTES.inspect} should never be used as part of attr_accessible.
  ERROR
  # We want to examine method calls. Particularly those that are calling the attr_accessible method
  # and also have arguments we care about
  def on_send(node)
    if receiver_attr_accessible?(node) && protected_arguments?(node)
      # If we do detect an attr_accessible call with arguments we care about, we can record an offense
      add_offense(node, message: ERROR_MESSAGE)
    end
  end
  private
  def receiver_attr_accessible?(node)
    node.method_name == :attr_accessible
  end
  def protected_arguments?(node)
    node.arguments.any? do |argument|
      if argument.sym_type? || argument.str_type?
        PROTECTED_ATTRIBUTES.include?(argument.value.to_sym)
      end
    end
  end
end

This custom cop does the trick. Adding a test for it ensures that it won't break in the future when we update RuboCop or extend the functionality. In order to write a test, we need to understand how the custom cops are set up and run.

Instantiate a custom cop

RuboCop::Cop::Cop inherits from RuboCop::Cop::Base and that allows the instantiation without any arguments. So it turns out this isn't anything special — creating a new instance of our cop is really as simple as: RuboCop::Cop::ProtectedAttrAccessibleFields.new

If the cop requires some kind of configuration, it can be passed to the instance via a RuboCop::Config object. The RuboCop::Config takes two arguments. RuboCop can provide configuration via YML files. You can use the first argument of RuboCop::Config to pass this configuration with various values from the test. The second argument is the path of the loaded YML file, which can be ignored in the tests.

config = RuboCop::Config.new({ RuboCop::Cop::ProtectedAttrAccessibleFields.badge.to_s => {} }, "/")
cop = RuboCop::Cop::ProtectedAttrAccessibleFields.new(config)

Process, execute, examine

As it turns out, there is a method available, RuboCop::Cop::Base#parse , that accepts a string as input and will return something the cop can process.

This allows us to have something like:

source = <<~CODE
  attr_accessible :account_id
CODE
processed_source = cop.parse(source)

There is a class from within RuboCop, RuboCop::Cop::Commissioner , that is responsible for taking a list of cops and using those to investigate the processed source code. In order to run our cop, we can run this method.

commissioner = RuboCop::Cop::Commissioner.new([cop])
investigation_report = commissioner.investigate(processed_source)

The RuboCop::Cop::Commissioner#investigate method will return an instance of RuboCop::Cop::Commissioner::InvestigationReport which is a simple struct class that has a list of offenses that have been recorded.

Put it all together

We end up with a test file that looks something like this:

describe RuboCop::Cop::ProtectedAttrAccessibleFields do
  let(:config) { RuboCop::Config.new({ described_class.badge.to_s => {} }, "/") }
  let(:cop) { described_class.new(config) }
  let(:commissioner) { RuboCop::Cop::Commissioner.new([cop]) }
  it "records an offense if we use allow account_id as a string" do
    source = <<~CODE
      attr_accessible :foo, 'account_id'
    CODE
    investigation_report = commissioner.investigate(cop.parse(source))
    expect(investigation_report.offenses).to_not be_blank
    expect(investigation_report.offenses.first.message).to eql described_class::ERROR_MESSAGE
  end
  it "records an offense if we use allow account_id as symbol" do
    source = <<~CODE
      attr_accessible :foo, :account_id
    CODE
    investigation_report = commissioner.investigate(cop.parse(source))
    expect(investigation_report.offenses).to_not be_blank
    expect(investigation_report.offenses.first.message).to eql described_class::ERROR_MESSAGE
  end
  it "doesn't record an offense if no protected attribute is used" do
    source = <<~CODE
      attr_accessible :foo
    CODE
    investigation_report = commissioner.investigate(cop.parse(source))
    expect(investigation_report.offenses).to be_blank
  end
end

Now that we know how to write tests, we can use them as a starting point for building new cops, extending existing cops, and ensuring that things continue to function as our application grows and evolves. These little investments into project-specific cops can end up being a large investment in the future health of the projects.

Sign up for a free trial of Aha! Develop

Aha! Develop is a fully extendable agile development tool. Prioritize the backlog, estimate work, and plan sprints. If you are interested in an integrated product development approach, use Aha! Roadmaps and Aha! Develop together. Sign up for a free 30-day trial or join a live demo to see why more than 5,000 companies trust our software to build lovable products and be happy doing it.

Making background jobs more resilient by default

Kyle d'Oliveira — Thu, 21 Jul 2022 20:31:11 +0000

When it comes to job processing, timing is everything. Running jobs in the background helps us remove the load from the web servers handling our customer's requests. However, we also want the background jobs to run in a reasonable amount of time for our customers. But what if a customer added so many background jobs that they used all of the background worker resources?

The jobs will be processed by available workers in the order they come in, which works great for the customer that added these jobs. But unfortunately for other accounts, their segments won't refresh until the backlog of all of those jobs is finished. This could be a problem for those accounts — it would look like the system isn't functioning properly as it waits for the queue to clear.

We have solved this problem at Aha! by creating a module that:

Batches separate jobs together
Limits the runtime of any single job
Limits parallel processing

When a job utilizes this new module, it becomes resilient to this problem by default.

Let's explore this with an example to show how to prevent this slowdown for all accounts. Let's say we have Accounts with various Segments (i.e., "groups"). Whenever we update a Segment, we want to refresh that segment. This process could take a bit of time so we will put it into a background job. We may end up with some controller code that looks like this:

class SegementsController < ApplicationController
  def update
    segment = current_account.segments.find(params[:id])
    segment.update!(segment_params)
    SegmentRefresher.perform_later(segment)
    head :ok
  end
end

It is usually good to process what can be done immediately and then delay some of the more expensive work. This allows us to give immediate responses back to our customers and still perform the work that needs to be done. However, a customer could put thousands of SegmentRefresher jobs onto our background job queue and prevent other customers' jobs from running.

Batching separate jobs together

Typically, when a background job is enqueued via ActiveJob, the parameters for the job are passed in as arguments to perform_later. This isn't quite what we want in order to batch jobs together. Instead, we create a new method perform_batch_later that puts the arguments into a data store such as Redis from which the job can later retrieve them.

So previously the job code may have looked like the following:

class SegmentRefresher < ApplicationJob
  def perform(segment)
    # Refresh the segment
  end
end

We now have something that looks like this:

module BatchByAccount
  extend ActiveSupport::Concern

  class_methods do
    # Push the data into Redis and then enqueue the job
    def perform_batch_later(data)
      data.each_slice(100) do |slice|
        Redis.current.rpush(data_key, slice.map(&:id))
      end
      perform_later
    end

    def self.data_key
      "SegmentRefresher:#{Account.current.id}"
    end
  end
end

class SegmentRefresher < ApplicationJob
  include BatchByAccount

  def perform
    segment_ids = Redis.current.lpop(self.class.data_key, 100)
    Segment.where(id: segment_ids).each do |segement|
      # Refresh the segment
    end
  end
end

The controller can now call SegmentRefresher.perform_batch_later with one or more Segments and that will be stored in Redis. Later, the job will run and grab 100 of those segment ids at a time to process.

This technique can be really powerful. It allows multiple processes to not know about the other and still batch the data together. Further, we can utilize different Redis methods to get slightly different behaviors. For example:

Using rpush to add records and lpop to remove them will give us a first in/first out queue. We can use this when the order of the jobs is important.
Using spush to add records and spop to remove them will give us an unordered set. This means that duplicate data is automatically filtered out, which prevents unnecessary work from being done. However, it is unordered so the jobs may be processed in a different order than they were enqueued.
Using a timestamp and zadd to add records and zrangebyscore / zrem to remove them lets us create a delayed unordered set. This is useful for actions we want to perform in the future. This might show up if we want to perform an action five minutes after a customer stops interacting with an object.

Limiting the runtime of any single job

Now that we are batching data together, we want to limit how long a single job can run. In order to tackle this, we leveraged functionality from the job-iteration gem. This gem provides an interface where we can define an enumerator and what to do each iteration. The gem will handle the rest.
Utilizing this, our job and module will now look like this:
(For ease of reading, the bit of code already shown has been removed.)

module BatchByAccount
  extend ActiveSupport::Concern

  class_methods do
    def perform_batch_later(data)
      # ...
    end

    def self.data_key
      # ...
    end
  end

  included do
    include JobIteration::Iteration
  end

  def build_enumerator(*)
    Enumerator.new do |yielder|
      # We will pull 100 records out of the queue at a time and yield that to the enumerator
      while (segment_ids = Redis.current.lpop(self.class.data_key, 100)).any?
        yielder.yield segment_ids, nil
      end
    end
  end

end

class SegmentRefresher < ApplicationJob
  include BatchByAccount

  def each_iteration(segment_ids)
    Segment.where(id: segment_ids).each do |segement|
      # Refresh the segment
    end
  end
end

Notice that the SegmentRefresher 's perform method has been swapped for an each_iteration method.

As long as there is data in Redis, this job will continue to perform until we hit the time threshold as defined by JobIteration.max_job_runtime. The default is five minutes. Once we hit the threshold, the job will be interrupted and will re-queue itself. This will ensure that even if it takes a long time to refresh all of the segments, it won't monopolize a worker.

Limiting parallel processing

Now that data is batched together and individual jobs are handling things in batches, we want to prevent race conditions of multiple jobs running at once. We solved this by using the activejob-uniqueness gem.

With this gem, we can make the jobs unique. Duplicate jobs for a single account will be ignored. Because there is only one job, we have to handle a race condition of what would happen if our one job finished at the same moment that new data is added.

The resulting job ends up looking like this:

module BatchByAccount
  extend ActiveSupport::Concern

  class_methods do
    def perform_batch_later(data)
      # ...
    end

    def self.data_key
      # ...
    end
  end

  included do
    include JobIteration::Iteration
    unique :until_expired, lock_ttl: 5.minutes

    rescue_from(StandardError, with: :handle_error)

    on_shutdown do
      # Ensure than when we are interrupting the job, that we clear the lock so that it can be re-queued
      lock_strategy.unlock(resource: lock_key)
    end

    on_complete do
      if Redis.current.llen(self.class.data_key) > 0
        # This is the race condition
        # If we are complete, but there is still data in the queue, we need to enqueue a new job to process it
        self.class.perform_later
      end
    end
  end

  end

  def handle_error(exception)
    # Ensure we unlock the job on error
    lock_strategy.unlock(resource: lock_key)
    raise exception
  end

  # These arguments can be tweaked or overridden to lock on different criteria or allow
  # some amount of parallelism
  def lock_key_arguments
    [Account.current.id]
  end

  def build_enumerator(*)
    # ...
  end
end

class SegmentRefresher < ApplicationJob
  include BatchByAccount

  def each_iteration(segment_ids)
    # ...
  end
end

Resiliency by default

The job itself barely changed but it is now more resilient. By creating some easy-to-reuse patterns, engineers can focus more on their own features instead of worrying about common resiliency problems. We can put energy into making the right choice easy for everyone.

The final code will look like this:

module BatchByAccount
  extend ActiveSupport::Concern

  class_methods do
    def perform_batch_later(data)
      data.each_slice(100) do |slice|
        Redis.current.rpush(data_key, slice.map(&:id))
      end
      # If the job is already enqueued or running, this will be a no-op
      perform_later
    end

    def self.data_key
      "SegmentRefresher:#{Account.current.id}"
    end
  end

  included do
    include JobIteration::Iteration
    unique :until_expired, lock_ttl: 5.minutes

    rescue_from(StandardError, with: :handle_error)

    on_shutdown do
      # Ensure than when we are interrupting the job, that we clear the lock so that it can be re-queued
      lock_strategy.unlock(resource: lock_key)
    end

    on_complete do
      if Redis.current.llen(self.class.data_key) > 0
        # This is the race condition
        # If we are complete, but there is still data in the queue, we need to enqueue a new job to process it
        self.class.perform_later
      end
    end
  end

  end

  def build_enumerator(*)
    Enumerator.new do |yielder|
      while (segment_ids = Redis.current.lpop(self.class.data_key, 100)).any?
        yielder.yield segment_ids, nil
      end
    end
  end

  def handle_error(exception)
    # Ensure we unlock the job on error
    lock_strategy.unlock(resource: lock_key)
    raise exception
  end

  # These arguments can be tweaked or overridden to lock on different criteria or allow
  # some amount of parallelism
  def lock_key_arguments
    [Account.current.id]
  end

end

class SegmentRefresher < ApplicationJob
  include BatchByAccount

  def each_iteration(segment_ids)
    Segment.where(id: segment_ids).each do |segement|
      # Refresh the segment
    end
  end
end