[ovs-dev] [PATCH] ovsdb-idl: Correct singleton insert logic

Mark Michelson mmichels at redhat.com
Mon Jun 25 12:08:02 UTC 2018

Hi Ben,

This fixes an issue that is being experienced by users in version 2.9 as 
well. Can this please be backported there as well?

Mark Michelson

On 06/15/2018 07:27 PM, Ben Pfaff wrote:
> On Tue, May 29, 2018 at 09:15:52AM -0400, Mark Michelson wrote:
>> Hi Ben,
>> Thanks for having a look and providing an update. See my replies in-line
>> below.
>> On 05/25/2018 06:31 PM, Ben Pfaff wrote:
>>> On Thu, May 17, 2018 at 01:16:55PM -0400, Mark Michelson wrote:
>>>> When inserting data into a "singleton" table (one that has maxRows ==
>>>> 1), there is a check that ensures that the table is currently empty
>>>> before inserting the row. The intention is to prevent races where
>>>> multiple clients might attempt to insert rows at the same time.
>>>> The problem is that this singleton check can cause legitimate
>>>> transactions to fail. Specifically, a transaction that attempts to
>>>> delete the current content of the table and insert new data will cause
>>>> the singleton check to fail since the table currently has data.
>>>> This patch corrects the issue by keeping a count of the rows being
>>>> deleted and added to singleton tables. If the total is larger than zero,
>>>> then the net operation is attempting to insert rows. If the total is
>>>> less than zero, then the net operation is attempting to remove rows. If
>>>> the total is zero, then the operation is inserting and deleting an equal
>>>> number of rows (or is just updating rows). We only add the singleton
>>>> check if the total is larger than zero.
>>>> This patch also includes a new test for singleton tables that ensures
>>>> that the maxRows constraint works as expected.
>>>> Signed-off-by: Mark Michelson <mmichels at redhat.com>
>>> Good catch.  (It's kind of weird to delete and repopulate a singleton
>>> table, but we should support it.)
>> The use case that brought this to our attention was the "set-ssl" commands
>> for ovn-nbctl and ovn-sbctl. Since the SSL table is a singleton, these
>> operations delete the current row and repopulate with a new row. The
>> existing check resulted in failure when when the transaction should have
>> been allowed.
>>> I think that the following patch achieves the same end with less
>>> bookkeeping overhead.  What do you think?  It doesn't break anything in
>>> the testsuite, but I didn't check that it actually achieves the purpose.
>> I did some testing and debugging and this looks like it has the expected
>> behavior, and I agree that this seems like a simpler approach. So
>> Acked-by: Mark Michelson <mmichels at redhat.com>
> Thanks.  I applied this to master.

More information about the dev mailing list