Clinton
Clinton

Reputation: 23135

Choosing appropriate locking:

Lets say I have a table T, with the following columns:

create table T as
(
  supplier varchar2,
  item varchar2,
  price number,
  is_best_price number,
);

And we have a procedure to insert items:

create or replace procedure insert_item
  (p_supplier as varchar2, p_item as varchar2, p_price as number) as
declare
  v_best_price;
  v_is_best_price number;
begin
  select min(price) into v_best_price from T where item = p_item;

  if (v_best_price is null) then
    v_is_best_price := 1;
  elsif (price <= v_best_price) then
    v_is_best_price := 1;
  else
    v_is_best_price := 0;
  end if;

  if (v_is_best_price = 0) then
    update T set is_best_price = 0 where item = p_item and is_best_price = 1;
  end if;

  insert into T values (p_supplier, p_item, p_price, v_is_best_price);
end;

The invariant here is that

Forall rows x: (x.v_best_price = 1) iff (x.v_price = select min(price) from T where item = x.item)

Or in plain english, is_best_price is 1 if the price for the item is the best price.

The problem occurs is if I do this in two different sessions:

insert_item('alice', 'pants', '30');
insert_item('bob', 'pants', '20');

Now, if I understand correctly, what could happen is the following:

(1) Execute insert_item('alice', 'pants', '30') (thread A)
(2) Execute insert_item('bob', 'pants', '20') (thread B)
(3) Thread A queries table, notices there are no other pants, so sets v_is_best_price := 1.
(4) Thread B queries table, notices there are no other pants (as thread A hasn't inserted yet), so sets v_is_best_price := 1.
(5) Thread A does insert ('alice', 'pants', '30', 1).
(6) Thread B does insert ('bob', 'pants', '20', 1).

We have violated our invariant.

So I realise I can lock the entire table in the first line of the procedure before the select by doing the following:

lock table T in exclusive mode;

Which if I understand correctly, would mean that any reads or writes to the table would be stopped until thread A completes (i.e. thread A and thread B could not run in parallel).

Is there any other way to do this however other than locking the entire table? Does select ... for update help? Or is there any other way to do finer grain locking?

I'm on Oracle 10g if that makes any difference.

Upvotes: 3

Views: 101

Answers (2)

Keith Irwin
Keith Irwin

Reputation: 5668

Your insert procedure doesn't unset the is_best_price flag for any other rows. So as written, it isn't going to maintain your invariant anyway. Before we start talking about solving the race condition, we need to fix that. In this case, doing so will lead us to a solution for the race condition.

There are several ways to do this update. One option is to blank all of the others when you set that is_best_price to 1 (in between the ELSIF and the ELSE), but that wouldn't handle ties.

Another option is to do this:

UPDATE T SET is_best_price = (price == v_best_price) WHERE item = p_item;

This can be done anywhere after the IF part. (Although the above code requires handling the v_best_price is NULL case by setting v_best_price to the price of the current item before running the UPDATE statement).

Once you're running this, though, you can just as easily run it after the INSERT as before and just insert everything with an is_best_price value of 0, thus removing the IF part entirely. (That is, change the last line to: insert into T values (p_supplier, p_item, p_price, 0); ) Doing so removes the NULL case from before because you know that you've always got at least one item (unless price can be NULL, but in that case, this will just result in is_best_price being 0 for any NULL priced items, including the one currently being inserted).

So, since we've gotten this far, there's now no point to having the SELECT which gives us v_best_price be at the front of the function. It serves no purpose because we've removed the IF part. So instead, we can just run

UPDATE T SET is_best_price = (price = (SELECT MIN(price) FROM T WHERE item = p_item)) WHERE item = p_item;

This will work properly in all cases. If another thread has inserted a cheaper item in the mean time, it will work fine. If it hasn't, likewise. As long as the UPDATE runs atomically (which it should), then no problems will arise, no extra locking needed. The only downside of this is that sometimes the UPDATE will be run twice when the table hasn't changed in the meantime, but that will happen in those cases with any corrected version of your old scheme as well, so this is a strict improvement.

Upvotes: 1

Andrew
Andrew

Reputation: 27294

Yes, its a race condition that allows both entries to consider they have the best price. You either have to come up with a convoluted scenario to use a unique index (in which case the 2nd insert will fail on commit) or reconsider whether you should store this flag.

Whilst it is a nice flag to have and convienient, it is also a bit wrong.

Imagine I insert a row that undercuts your existing price on an item, the logic in your code block doesnt set the other rows is_best_price to 0 - it just is making a decision about what to do on the row being added, so I can get two rows with a 1 for the same item, regardless of race condition.

If you can afford the processing overhead, calculate which is the best on the fly when you need it, instead of trying to maintain those values as you go along.

Upvotes: 2

Related Questions