On Thu, Jan 27, 2022 at 10:00 AM Dumitru Ceara <dce...@redhat.com> wrote: > > We still need to try to ovsdb_idl_loop_commit_and_wait() on instances > that are in standby mode. That's because they need to try to take the > lock. But if they're in standby they won't actually build a transaction > json and will return early because they don't own the SB lock. > > That's reported as an error by ovsdb_idl_loop_commit_and_wait() but we > shouldn't act on it. > > Also, to ensure that no DB changes are missed, ovsdb_idl_track_clear() > should be called only on active instances. The standby or paused ones > will get the incremental updates when they become active. Otherwise we > would be forced to perform a full recompute when the instance becomes > active.
Hi Dumitru, I've a question on the track clear being moved out of the standby instances. To ensure correctness, I suppose it's better to trigger a full recompute when a standby instance becomes active. What do you think? Also lets say CMS does the below operations - Add a logical switch S1 - Add a logical port p1 in S1 - Add a logical port p2 in S1 - Delete logical port p2 - Delete a logical switch. With this patch since we are not clearing the tracking information, how does ovn-northd process the tracked changes when it becomes active ? Thanks Numan > > Fixes: 953832eb17f3 ("ovn-northd: Don't trigger recompute for transactions > that are in progress.") > Fixes: 4597317f16d1 ("northd: Introduce incremental processing for northd") > Reported-by: Ilya Maximets <i.maxim...@ovn.org> > Signed-off-by: Dumitru Ceara <dce...@redhat.com> > --- > northd/ovn-northd.c | 35 ++++++++++++++++++++--------------- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 793135ede..4e1e51931 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -1089,6 +1089,26 @@ main(int argc, char *argv[]) > ovnnb_txn, ovnsb_txn, > &ovnsb_idl_loop); > } > + > + /* If there are any errors, we force a full recompute in > order > + * to ensure we handle all changes. */ > + if (!ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop)) { > + VLOG_INFO("OVNNB commit failed, " > + "force recompute next time."); > + recompute = true; > + } > + > + if (!ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop)) { > + VLOG_INFO("OVNSB commit failed, " > + "force recompute next time."); > + recompute = true; > + } > + ovsdb_idl_track_clear(ovnnb_idl_loop.idl); > + ovsdb_idl_track_clear(ovnsb_idl_loop.idl); > + } else { > + /* Make sure we send any pending requests, e.g., lock. */ > + ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop); > + ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop); > } > } else { > /* ovn-northd is paused > @@ -1112,21 +1132,6 @@ main(int argc, char *argv[]) > ovsdb_idl_wait(ovnsb_idl_loop.idl); > } > > - /* If there are any errors, we force a full recompute in order to > - ensure we handle all changes. */ > - if (!ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop)) { > - VLOG_INFO("OVNNB commit failed, force recompute next time."); > - recompute = true; > - } > - > - if (!ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop)) { > - VLOG_INFO("OVNSB commit failed, force recompute next time."); > - recompute = true; > - } > - > - ovsdb_idl_track_clear(ovnnb_idl_loop.idl); > - ovsdb_idl_track_clear(ovnsb_idl_loop.idl); > - > unixctl_server_run(unixctl); > unixctl_server_wait(unixctl); > memory_wait(); > -- > 2.27.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev