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

Ben Pfaff blp at ovn.org
Mon Jun 25 20:22:34 UTC 2018


Done.

On Mon, Jun 25, 2018 at 08:08:02AM -0400, Mark Michelson wrote:
> 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?
> 
> Thanks,
> 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