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