I don't get how that race condition happened in my app which has a single end user

Chat / Discussions
, ,
1

Hi,

I have a web app using Phoenix live view on production it has one single end user. It’s an order system. If you like to have more idea about it you may visit here where I asked a question earlier.

One of my mistake, I had to forget to create a unique constraint for customer’s phone_number. However my code was like this:

def get_or_create_customer_by_phone_number(phone_number) do
    case Repo.get_by(Customer, phone_number: phone_number) do
      nil ->
        case create_customer(%{:phone_number => phone_number}) do
          {:ok, customer} -> customer
          {:error, %Ecto.Changeset{} = changeset} -> {:error, changeset}
        end

      customer ->
        customer
    end
  end

With one single request, especially within 2 days it has created same number twice for 5 different numbers, so the 3rd order couldn’t be created because get_by expect one single record while it returns 2; it throws an error.

Here’s one prove from db query result:

myapp_prod=# select * from customers where phone_number = '437';
 id  | phone_number |     inserted_at     |     updated_at
-----+--------------+---------------------+---------------------
   5 | 437          | 2020-07-07 08:22:26 | 2020-07-09 12:32:08
 122 | 437          | 2020-07-09 12:01:51 | 2020-07-09 12:56:40
(2 rows)

myapp_prod=# SELECT o.id, oi.product_id, o.customer_id from Order_Items oi INNER JOIN Orders o on o.id = oi.order_id WHERE o.customer_id IN (5, 122);
 id  | product_id | customer_id
-----+------------+-------------
 126 |          1 |         122
   5 |          1 |           5
(2 rows)

myapp_prod=# update orders set customer_id = 5 where id = 126;
UPDATE 1
myapp_prod=# delete from customers where id = 122;
DELETE 1

So, after I read some documentation about race conditions, I had to drop the index for the phone_number and created a unique index to prevent it.

defmodule MyApp.Repo.Migrations.CreateUniqueConstraintForCustomer do
  use Ecto.Migration

  def change do
    drop index(:customers, [:phone_number])
    create unique_index(:customers, [:phone_number])
  end
end

then I changed to my code according to the documentation like this:

  def get_or_create_customer_by_phone_number(phone_number) do
    %Customer{}
    |> Ecto.Changeset.change(phone_number: phone_number)
    |> Ecto.Changeset.unique_constraint(:phone_number)
    |> Repo.insert
    |> case do
      {:ok, customer} -> customer
      {:error, _} -> Repo.get_by(Customer, phone_number: phone_number)
    end
  end

But still I do want to know how that happened? Any idea guys?

Thanks!

2

Is the function above only thing that can create a customer? The inserted at column is 2 days apart, so it’s not a race condition from a single user. I’d expect to see one that is milliseconds apart.

2 Likes
3

It could be something as simple as resending the creation request.

The main problem is expecting phone number to be unique without actually enforcing the constraint

1 Like
4

No, there’s standard Phoenix created form but they don’t use there, for the quick reaction it creates customer during order request. But yes I can say it’s the only place that create the customer since the user use only Order Request form.

he says I don’t click for the second time until the progress finish (there’s a request progress on the top the page, the standard Phoenix Live View’s blue bar) finishes. He said, the button says “Saving …” but it’s not … maybe there’s another problem but this is also another problem creating multiple user (sometimes), I just got the idea of race condition is from here. So, okay that’s not a race condition problem actually, thanks @sb8244.

that’s true.

5

Hmm. Is OP sure to be redirecting from a POST? Not accepting a GET request, and then not redirecting, such that bookmarking the newly-created account then using the bookmark would try to create again?

6

As @sb8244 mentioned, the 2nd record is inserted 2 days later, and looking at the id value (which I presume is of the serial type) I’m further inclined to suspect that these records have not been inserted close to each other, so for the moment I’d rule out race condition.

Moreover, looking at updated_at we can tell that the first record has been updated after the second one was inserted, so one possibility that there’s a bug in the update logic (e.g. the user changed the number to 437, and the update logic didn’t check that such number already exists).

Either way, a unique constraint is the correct solution here, because it guards the integrity of the data, and it should help you find the cause of the problem.

1 Like
7

The only way update the customer is when the order delivered, it sets the location information which is in the same table.

  schema "customers" do
    field :phone_number, :string
    field :address_lat, :float
    field :address_lon, :float

    timestamps()
  end

but yeah there’s something wrong for sure.