Andy Zhou <az...@ovn.org> wrote on 09/01/2016 02:02:32 AM: > > On Fri, Jan 8, 2016 at 8:58 AM, Liran Schour <lir...@il.ibm.com> wrote: > Andy Zhou <az...@ovn.org> wrote on 07/01/2016 01:04:17 AM: > > > > I have some comments on the patch series. > > > > Instead of "monitor_cond_change", why not just have a more generic > > "monitor_update" message, so we can update > > monitor columns as well as conditions. > > > > In case of changing conditions, there is a real need for that. I was > getting to it from the perspective of OVN however I believe there are > others use cases for that functionality. In case of changing the monitored > columns, I am not sure there is a real need for that. If there is, we can > change "monitor_cond_change" to a more generic "monitor_updtae" that will > be able to change the monitored columns. However that will add complexity > to the code because columns unlike conditions are being held by > ovsdb_monitor and they are common to all jsonrpc monitor session's using > this ovsdb_monitor. Changing the columns in the middle will force us to > move this jsonrpc monitor session to a new ovsdb_monitor with the new > columns. In case of the conditions, they are specific per jsonrpc monitor > session from the start. > If there is a real need for changing the monitored columns I can add this > functionality to the patch series. Tell me what do you think?
> Purely from protocol stand point, it would be nice to avoid > introducing new methods > if we can avoid it. If there is not an immediate need for column > changes, we can > restrict our implementation to condition changes only to begin > with. The implementation > can simply nack any monitor_updates that contains non-empty > <monitor-request>. > I still think there is a difference between the conditions and columns and we should not allow changing columns during session like we trying to allow regarding to conditions. Columns is a more static element, we know all the columns from the start. In my opinion if a user wants to change the monitored columns it should cancel the existing monitor session and start a new one with the new columns. It is different in case of conditions. It is a trivial use case to iteratively change the conditions to change the monitored rows. Conditions are a more dynamic issue the columns. Anyhow it seems like a specification matter and we already had a review over the spec in the past. Maybe we should raise this question to re-approve the spec before I will implement it. What do you think? > > The goal is for the client to be able tell if an update message are > generated before or after the condition changes. > To be precise, the monitor_id I mentioned refers to <json-value> > within the monitor message. > Same as above I can implement this but maybe we should have a broader agree about it. > In addition, it may be good to explicitly define the timing > requirements for monitor updates. Is it reasonable to require > New updates be sent only after the monitor updates messages are acked? > Will document it in the man page. > > Should any columns within schema be allowed in condition? or only > > the ones being monitored? > > From protocol viewpoint, it would make sense to allow any column. > > > > Yes, any columns should be allowed in condition. > Perfect. It will be nice to make this clear in the ovsdb-server man page. > > > I see that conditions are applied when generating updates. Could > > this lead to inconsistency when fast and slow connections receives > > different content > > when condition changes? > > > > During the request for conditions change, A new change list is being > recorded holding all the rows of the table with the changes accord since > the last flush. This change list will be sent to the client on update > notification and only then condition evaluation will take place. I do not > see an inconsistency possibility here. Do you see one? > Sorry, you are right, there is no race condition here. I missed > that conditions are evaluated for > each Jsonrpc connection separately. On the other hand, Would it be > nice for monitors with the same > conditions not have to evaluate the same conditions multiple times? > After submitting the basic functionality I plan to implement a cache that will be indexed by condition's clauses and will reduce the need to evaluate conditions per row. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
