On Thu, Apr 29, 2021 at 9:50 AM Ilya Maximets <[email protected]> wrote: > > On 4/29/21 5:50 PM, Han Zhou wrote: > > > > > > On Thu, Apr 29, 2021 at 3:12 AM Dumitru Ceara <[email protected] <mailto:[email protected]>> wrote: > >> > >> On 4/29/21 11:29 AM, Ilya Maximets wrote: > >> > On 4/29/21 10:07 AM, Dumitru Ceara wrote: > >> >> On 4/29/21 3:55 AM, Han Zhou wrote: > >> >>> This reverts commit 0ba2e1a2e2c413f057ac198e4c2bcf00ac40b19f. > >> >> > >> >> Oops, I should've thought about this when reviewing the initial patch, > >> >> good that the CI caught it. > >> >> > >> >>> > >> >>> System tests with "dp-groups=yes" are all failing due to the warning > >> >>> log. Need to investigate why the warning appears. But before that is > >> >>> clear, revert this log level change to make CI pass. > >> >> > >> >> I would remove this part of the commit log or change it to something like: > >> >> > >> >> "With conditional monitoring, if a logical flow refers to a datapath > >> >> group that doesn't contain any local datapaths, ovsdb-server will send > >> >> updates with lflow.logical_dp_group set to NULL. > >> > > >> > This is not correct. lflow does have a logical_dp_group in a message > >> > received from the ovsdb-server. It's IDL that hides it from a user > >> > because it didn't receive the group itself. And it's allowed to do > >> > that because of the 'min:0, max:1' definition of the column in the schema. > >> > >> You're right, thanks for the correction. > >> > >> > > >> >> This is a valid case > >> >> and we shouldn't log a warning message for it." > >> > > >> > +1 > >> > It's a valid case and should not be logged as a warning. > >> > > >> >> > >> >> It's actually expected to have lflows with both logical_dp_group and > >> >> logical_datapath NULL now since we unconditionally monitor lflows with > >> >> datapath group set. > >> > > >> > Another point here is that we can't actually guarantee the order of updates > >> > from the server. Even though currently ovsdb-server sends updates for > >> > monitored tables in a single 'update' message, it's not mandatory and could > >> > change in the future. In this case it will be possible to receive lfow > >> > before receiving dp_group even with monotor-all. > >> > >> +1 > >> > >> So even with the additional patch Han is preparing (to unconditionally > >> monitor all DPGs) we should still revert this change. > >> > > > > I think it should be guaranteed by OVSDB transaction that all changes in the same transaction should be sent and received atomically, as long as the rows satisfy the monitor condition. Otherwise, ovsdb transaction is not really useful. > > Transactions and monitor updates are separate things. They just happen > to occur sometimes on the same connection. Transaction itself is atomic, > i.e. it's executed as a whole or not at all. But between sending the > transaction request and receiving transaction response client may receive > arbitrary number of database updates that might be or might not be related > to the transaction. As long as these updates are consistent (i.e. doesn't > violate database integrity), there is no reason to send them within a > single 'update' message. > > ovsdb-server guarantees that transaction response will be sent > after all the monitor updates; and only after receiving of that response, > client can be sure that it has an up to date version of a database with > all the changes it made. > > For example, client A sends transaction with changes X and Y. Client > B at the same time sends transaction with change X only. After that > both clients receives update with change X, because transaction from > B happened to hit ovsdb-server first. Now assuming that X was an > unconditional mutation of a row, i.e. had no prerequisites. > When transaction from client A hits ovsdb-server it will not be rejected > because X has no prerequisites. X will be executed and Y will be > executed. At this point both clients will receive monitor update > with only changes made by operation Y, because X didn't actually > change anything as it mutated the row to exactly same row as it was. > After that client A receives the transaction response with success. > > Summarizing: > 1. A sends transaction {X, Y}. > 2. A receives update for X. > 3. A receives update for Y. > 4. A receives transaction response. > > And this can happen right now with current implementation of ovsdb-server. > Thanks for the example. I am actually focusing on how a client should observe the changes made by others. I remember that it is guaranteed that they won't see a partial update of a single transaction (unless the monitor condition is only interested in part of the updates). If you are talking about how would a client see its own update, I believe it is the same principle but if it happens to make same changes as another client transaction, as your example has shown, then I think it makes sense that it may receive its own updates in two messages, but as you also mentioned this is not a completely correct use of the transaction - the could have set the prerequisite in the transaction.
For the DPG use case, we are talking about how ovn-controller should observe the changes made by ovn-northd, so I think this example is irrelevant, and monitor-all should guarantee the lflow always has DP or DPG, right? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
