On Thu, Apr 12, 2018 at 10:19 AM, Ben Pfaff <[email protected]> wrote: > > On Wed, Apr 11, 2018 at 11:29:28PM -0700, Han Zhou wrote: > > On Tue, Apr 10, 2018 at 6:21 PM, Han Zhou <[email protected]> wrote: > > > > > > > > > > > > On Tue, Apr 10, 2018 at 5:04 PM, Ben Pfaff <[email protected]> wrote: > > > > > > > > On Fri, Apr 06, 2018 at 02:40:21PM -0700, Han Zhou wrote: > > > > > On Fri, Apr 6, 2018 at 1:54 PM, Ben Pfaff <[email protected]> wrote: > > > > > > > > > > > > Thanks for working on making OVN faster and scale better. > > > > > > > > > > > > I see what you mean about how nb_cfg can be a scale problem. > > Really, > > > > > > each hypervisor only cares about its own row, but each of them is > > being > > > > > > notified about the current value in every row. Ideally, we want > > the HVs > > > > > > to be able to say to ovsdb-server, "Give me all the data in the > > Chassis > > > > > > table, except don't bother with nb_cfg in any rows except my own > > row." > > > > > > > > > > > > Actually, there's a way for ovn-controller to do that: the OVSDB > > > > > > protocol definition of monitor_cond allows the client to specify > > > > > > multiple sets of columns to monitor in a table, and each set of > > columns > > > > > > has an independently attached condition. This can be used to > > specify > > > > > > that most of the columns are monitored unconditionally but nb_cfg is > > > > > > monitored only for a particular row. > > > > > > > > > > > > But there are still problems: > > > > > > > > > > > > 1. The implementation in ovsdb-server looks broken. I don't think > > it > > > > > > implements the spec. It certainly isn't tested--none of the > > existing > > > > > > clients ever passes more than a single set of columns. I think > > that > > > > > > it will work if all the sets of columns use the same condition, > > but > > > > > > that doesn't help with this case. It will need to be fixed. > > > > > > > > > > > > 2. The client implementation in ovsdb-idl doesn't anticipate > > difference > > > > > > conditions for different columns. It will need to be enhanced to > > > > > > support this use. > > > > > > > > > > > > In the short term this is going to be more work than just creating > > a new > > > > > > table. In the long term, though, it will allow us to be more > > flexible > > > > > > and more agile, since improvements similar to the one in this patch > > will > > > > > > require only client changes, not database schema changes. > > > > > > > > > > > > Comments? > > > > > > > > > > Thanks for the valuable information! I didn't even know that ovsdb is > > > > > supposed to support multiple set columns with different monitor > > conditions. > > > > > It surely would be the right way to go if it is supported. However, > > as you > > > > > mentioned the mechanism doesn't work yet and I am not sure how much > > work is > > > > > needed to make it work, it seems reasonable to me to "workaround" it > > at > > > > > least for short term until that is ready in the future. Then we could > > > > > simply resurrect the old column in the old table with the new monitor > > > > > conditions. What do you think? > > > > > > > > This kind of workaround would essentially persist forever. We'd never > > > > be able to get rid of it--or, more to the point, we would never be able > > > > to justify the work. We'd be stuck with something ugly. > > > > > > > > So: if you think the workaround is valuable, then one way to justify it > > > > would be to reformulate it in a way that isn't so ugly. As currently > > > > written, when a person looks at the table structure, (s)he notices a > > > > table that has a single column and is named for that single column. > > > > That sticks out, clearly, as some kind of compromise, and the > > > > documentation even calls it out as an optimization. But there might be > > > > a logical, conceptual reason why it makes sense to have a separate table > > > > for some kinds of chassis-related data. If there is, then that would > > > > suggest a name for the table, and for the class of data that should > > > > should go in there, and it would make it possible to have something that > > > > is both faster and a reasonable design. > > > > > > > > I'm not saying that there is necessarily a logical, conceptual reason > > > > that one can come up here, but if there is then it would certainly make > > > > it much easier to support the patch. > > > > > > Hi Ben, I'm not sure if I got your point completely. A logical reason for > > this separate table might be for holding chassis private data, the data > > that other chassis is not interested in. Of course the reason for holding > > such private data in a separate table is still this huge performance > > difference as it is shown in the test result. So I think the table can be > > named as Chassis_Private_Data, and I can document in ovn-sb.xml more about > > the performance considerations, and the future improvement about the > > monitor_cond. Would this address your point or did I totally missed your > > point? > > > > > Hi Ben, could you confirm so that I can work on v3? > > > > > Thanks, > > > Han > > That sounds like a good direction.
I have sent the v3 some time ago: https://patchwork.ozlabs.org/patch/899608/ Please take a look. Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
