Search code examples
elixirphoenix-frameworkecto

Ecto.Repo.update_all for atomic updates?


There's a section in Phoenix's Contexts guide that adds page-view-incrementing functionality to a dummy CMS context. The function that's created in the CMS context looks like this:

def inc_page_views(%Page{} = page) do
  {1, [%Page{views: views}]} =
    from(p in Page, where: p.id == ^page.id, select: [:views])
    |> Repo.update_all(inc: [views: 1])

  put_in(page.views, views)
end

Paraphrasing, inc_page_views takes a Page struct, uses its id to look up the corresponding database record, uses Repo.update_all to atomically increment the view count (see docs for an interleaving example), ensures that only 1 record was updated, and returns a new Page with the update view count.

Why does this example use Ecto.Repo.update_all/3 rather than Ecto.Repo.update/2? Since we know we want to operate on only 1 record, it feels strange to potentially update a bunch of records and retroactively check that we didn't, rather than updating a specific Ecto.Changeset, which might look something like this:

def inc_page_views(%Page{views: curr_views} = page) do
  page
  |> Page.changeset(%{views: curr_views + 1})
  |> Repo.update()
end

This implementation is shorter/simpler, but I'm guessing the Phoenix doc writers didn't use it for a good reason. My hunch is that the Repo.update version must lack that atomic update property that's supposedly present in the Repo.update_all version, but I have no idea why! Can someone help explain the difference between these implementations and why the docs might have chosen the first?


Solution

  • def inc_page_views(%Page{views: curr_views} = page) do
      page
      |> Page.changeset(%{views: curr_views + 1})
      |> Repo.update()
    end
    

    it introduces a race condition. Imagine that you get the page from the database and it has views equal to 5. Then, while you are running the function above, the other db connection from the other process may change the value from 5 to 6. But since this function doesn't know about it, it will still add 1 to the 5 (now outdated value) and write to the database the value 6.

    As a result, instead of having the correct value 7, you have 6.

    The way to prevent that is to use database locks doing something like that:

    Page
    |> where(id: ^id)
    |> lock("FOR UPDATE")
    |> Repo.one!()
    |> inc_page_views()
    

    Or simply use the Repo.update_all that makes sure that the operation is atomic.