On Fri, Apr 01, 2016 at 02:14:27PM +0200, Liran Schour wrote: > Ben Pfaff <b...@ovn.org> wrote on 30/03/2016 07:56:01 PM: > > > On Tue, Mar 22, 2016 at 11:24:14PM +0200, Liran Schour wrote: > > > Ben Pfaff <b...@ovn.org> wrote on 22/03/2016 07:23:33 PM: > > > > > > > On Fri, Mar 04, 2016 at 08:08:59AM +0000, Liran Schour wrote: > > > > > Hold session's conditions in ovsdb_monitor_session_condition. Pass > it > > > > > to ovsdb_monitor for generating "update2" notifications. > > > > > Add functions that can generate "update2" notification for a > > > > > "monitor_cond" session. > > > > > JSON cache is enabled only for session's with true condition only. > > > > > "monitor_cond" and "monitor_cond_change" are RFC 7047 extensions > > > > > described by ovsdb-server(1) manpage. > > > > > > > > > > Signed-off-by: Liran Schour <lir...@il.ibm.com> > > > > > > > > I think that ovsdb_monitor_table_condition_set() has a memory leak > in > > > > the error case; it does not free the 'error' returned to it. > > > > > > > > > > Right. Will fix that. > > > > > > > I'm a little confused about ovsdb_monitor_session_condition. Why is > > > > this a separate data structure? That is, why is there a separate > shash > > > > of conditions rather than incorporating a condition into > > > > ovsdb_monitor_table? > > > > > > > > > > A single ovsdb_monitor instance can be used by many jsonrpc sessions > (as > > > long as they share the same set of tables and columns). > > > Ovsdb_monitor_condition_session is holding the condition state per > each > > > jsonrpc session. Each session, in ovsdb_monitor, can have a different > set > > > of conditions and it is being represented by > > > ovsdb_monitor_session_condition with ovsdb_monitor_table_condition per > > > > table. > > > The trivial option was to divide ovsdb_monitor into 2 different > instances > > > if we have 2 sessions with different sets of conditions. But, > condition is > > > changing during the lifetime of monitor session. And by holding > sessions > > > with different conditions under the same ovsdb_monitor we can reduce > the > > > computation needed to insert row changes to ovsdb_monitor. (insert > once > > > instead of twice) > > > > Those downsides of having a monitor per (monitored columns, condition) > > seem pretty minor. It also seems to me like the "obvious" way to > > implement this. It seems like, for example, the problem of caching with > > conditions is much easier if the data structures are organized this > > way. Did you actually try implementing it that way and determine that > > there was some insuperable difficulty? > > No, I did not try implementing it in the 'trivial' way. The reason is > twofold: first, to gain the ability to share the computed updates between > sessions, also as per-session conditions change; second, to gain the > ability to 'fall back' to using a Json cache for condition-less tables. (A > typical monitor session includes conditioned tables and condition-less > tables). > > My implementation choice achieves the above and has the complexity of > processing a DB update in O(#conditioned columns). The 'trivial' > alternative would not achieve the above in the general case and would have > the complexity of processing a DB update in O(#jsonrpc sessions with > matched conditions), in the best case. > > As I see it, the above stated benefits of the more complex design worth > the complexity increase in the code, given also that the memory complexity > is similar.
As I understand it, the implementation in this patch series cannot share a cache of changes across sessions that include a condition, even if the condition is the same. Is that correct? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
