On Wed, Feb 23, 2022 at 1:53 AM Dumitru Ceara <[email protected]> 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. > > Explicitly force a full recompute when transitioning from "standby" or > "paused" to "active". > > 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 <[email protected]> > Signed-off-by: Dumitru Ceara <[email protected]> > --- > v2: > - Address Han's and Numan's comments: > - Clear tracked changes on every run (on standby or paused instances) > and force a recompute when transitioning back to "active". > --- > northd/ovn-northd.c | 32 ++++++++++++++++++++++---------- > 1 file changed, 22 insertions(+), 10 deletions(-) > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 8a8c6d07db..c56b29f2a5 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -835,6 +835,27 @@ 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; > + } > + } 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); > + > + /* Force a full recompute next time we become active. */ > + recompute = true; > } > } else { > /* ovn-northd is paused > @@ -856,17 +877,8 @@ main(int argc, char *argv[]) > ovsdb_idl_run(ovnsb_idl_loop.idl); > ovsdb_idl_wait(ovnnb_idl_loop.idl); > 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."); > + /* Force a full recompute next time we become active. */ > recompute = true; > } > > -- > 2.27.0 >
Thanks Dumitru. Applied to main and branch-21.12. Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
