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 <mmich...@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 <mmich...@redhat.com>

Thanks.  I applied this to master.


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to