Although each individual VIF port related changes are handled incrementally, it still triggers recompute if there are in-flight transactions (either to NB or SB) when a change comes, which is very common in real production environment if change happens frequently. It is also easy to hit such situation in test cases where nb_cfg mechanism is heavily used, which makes it difficult to write reliable and stable tests, such as what the commit 8c30ba1386 was trying to work around.
This patch skips the I-P engine execution until the NB & SB transaction handles are available (no in-flight transactions), and when skippiing the runs it keeps the tracked changes in IDL across main loop iterations. This way we avoid recompute without worrying about missing any changes. Signed-off-by: Han Zhou <[email protected]> --- northd/inc-proc-northd.c | 6 +-- northd/ovn-northd.c | 22 +++++---- tests/ovn-northd.at | 104 +++++++++++++++++++-------------------- 3 files changed, 66 insertions(+), 66 deletions(-) diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 19fc67795643..d328deb222e6 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -296,6 +296,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, struct ovsdb_idl_txn *ovnsb_txn, bool recompute) { + ovs_assert(ovnnb_txn && ovnsb_txn); engine_init_run(); /* Force a full recompute if instructed to, for example, after a NB/SB @@ -312,10 +313,7 @@ bool inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn, }; engine_set_context(&eng_ctx); - - if (ovnnb_txn && ovnsb_txn) { - engine_run(true); - } + engine_run(true); if (!engine_has_run()) { if (engine_need_run()) { diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 3e0848e41b8e..4fa1b039ea32 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -881,6 +881,7 @@ main(int argc, char *argv[]) simap_destroy(&usage); } + bool clear_idl_track = true; if (!state.paused) { if (!ovsdb_idl_has_lock(ovnsb_idl_loop.idl) && !ovsdb_idl_is_lock_contended(ovnsb_idl_loop.idl)) @@ -930,25 +931,26 @@ main(int argc, char *argv[]) } if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) { - int64_t loop_start_time = time_wall_msec(); - bool activity = inc_proc_northd_run(ovnnb_txn, ovnsb_txn, - recompute); - recompute = false; - if (ovnsb_txn) { + bool activity = false; + if (ovnnb_txn && ovnsb_txn) { + int64_t loop_start_time = time_wall_msec(); + activity = inc_proc_northd_run(ovnnb_txn, ovnsb_txn, + recompute); + recompute = false; check_and_add_supported_dhcp_opts_to_sb_db( ovnsb_txn, ovnsb_idl_loop.idl); check_and_add_supported_dhcpv6_opts_to_sb_db( ovnsb_txn, ovnsb_idl_loop.idl); check_and_update_rbac( ovnsb_txn, ovnsb_idl_loop.idl); - } - if (ovnnb_txn && ovnsb_txn) { update_sequence_numbers(loop_start_time, ovnnb_idl_loop.idl, ovnsb_idl_loop.idl, ovnnb_txn, ovnsb_txn, &ovnsb_idl_loop); + } else if (!recompute) { + clear_idl_track = false; } /* If there are any errors, we force a full recompute in order @@ -998,8 +1000,10 @@ main(int argc, char *argv[]) recompute = true; } - ovsdb_idl_track_clear(ovnnb_idl_loop.idl); - ovsdb_idl_track_clear(ovnsb_idl_loop.idl); + if (clear_idl_track) { + ovsdb_idl_track_clear(ovnnb_idl_loop.idl); + ovsdb_idl_track_clear(ovnsb_idl_loop.idl); + } unixctl_server_run(unixctl); unixctl_server_wait(unixctl); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index f1bf9092eeb7..e79d33b2aec5 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -9522,67 +9522,65 @@ as hv1 ovs-vsctl add-br br-phys ovn_attach n1 br-phys 192.168.0.11 -fail_count=0 check_recompute_counter() { northd_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute) - if test x$northd_recomp != x$1; then - fail_count=$(($fail_count + 1)) - echo check northd recompute failed: expected $1, got $northd_recomp - return 1 - fi + AT_CHECK([test x$northd_recomp = x$1]) + lflow_recomp=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute) - if test x$lflow_recomp != x$2; then - fail_count=$(($fail_count + 1)) - echo check lflow recompute failed: expected $2, got $lflow_recomp - return 1 - fi - return 0 + AT_CHECK([test x$lflow_recomp = x$2]) } -# Depending on order of responses from NB and SB, the number of recompute may -# be different. This test case only verifies the best case scenario, which -# should have the expected recompute count at least 50% of the time. +check ovn-nbctl --wait=hv ls-add ls0 + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=hv lsp-add ls0 lsp0-0 -- lsp-set-addresses lsp0-0 "unknown" +ovs-vsctl add-port br-int lsp0-0 -- set interface lsp0-0 external_ids:iface-id=lsp0-0 +wait_for_ports_up +check ovn-nbctl --wait=hv sync +check_recompute_counter 5 5 + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=hv lsp-add ls0 lsp0-1 -- lsp-set-addresses lsp0-1 "aa:aa:aa:00:00:01 192.168.0.11" +ovs-vsctl add-port br-int lsp0-1 -- set interface lsp0-1 external_ids:iface-id=lsp0-1 +wait_for_ports_up +check ovn-nbctl --wait=hv sync +check_recompute_counter 0 0 + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=hv lsp-add ls0 lsp0-2 -- lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:02 192.168.0.12" +ovs-vsctl add-port br-int lsp0-2 -- set interface lsp0-2 external_ids:iface-id=lsp0-2 +wait_for_ports_up +check ovn-nbctl --wait=hv sync +check_recompute_counter 0 0 + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=hv lsp-del lsp0-1 +check_recompute_counter 0 0 + +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=hv lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:88 192.168.0.88" +check_recompute_counter 0 0 + +# Delete and re-add a LSP for several times continuously, to ensure +# frequent operations do not trigger recompute when there are in-flight +# transcations. for i in $(seq 10); do - check ovn-nbctl --wait=hv ls-add ls$i - - check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats - check ovn-nbctl --wait=hv lsp-add ls$i lsp${i}-0 -- lsp-set-addresses lsp${i}-0 "unknown" - ovs-vsctl add-port br-int lsp${i}-0 -- set interface lsp${i}-0 external_ids:iface-id=lsp${i}-0 - wait_for_ports_up - check ovn-nbctl --wait=hv sync - check_recompute_counter 5 5 || continue - - check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats - check ovn-nbctl --wait=hv lsp-add ls$i lsp${i}-1 -- lsp-set-addresses lsp${i}-1 "aa:aa:aa:00:00:01 192.168.0.11" - ovs-vsctl add-port br-int lsp${i}-1 -- set interface lsp${i}-1 external_ids:iface-id=lsp${i}-1 - wait_for_ports_up - check ovn-nbctl --wait=hv sync - check_recompute_counter 0 0 || continue - - check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats - check ovn-nbctl --wait=hv lsp-add ls$i lsp${i}-2 -- lsp-set-addresses lsp${i}-2 "aa:aa:aa:00:00:02 192.168.0.12" - ovs-vsctl add-port br-int lsp${i}-2 -- set interface lsp${i}-2 external_ids:iface-id=lsp${i}-2 - wait_for_ports_up - check ovn-nbctl --wait=hv sync - check_recompute_counter 0 0 || continue - - check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats - check ovn-nbctl --wait=hv lsp-del lsp${i}-1 - check_recompute_counter 0 0 || continue - - check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats - check ovn-nbctl --wait=hv lsp-set-addresses lsp${i}-2 "aa:aa:aa:00:00:88 192.168.0.88" - check_recompute_counter 0 0 || continue - - # No change, no recompute - check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats - check ovn-nbctl --wait=sb sync - check_recompute_counter 0 0 || continue + # wait for sb but not wait for hv + check ovn-nbctl --wait=sb lsp-del lsp0-2 + check ovn-nbctl --wait=sb lsp-add ls0 lsp0-2 -- lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:02 192.168.0.12" - CHECK_NO_CHANGE_AFTER_RECOMPUTE + # even without waiting for sb + check ovn-nbctl lsp-del lsp0-2 + check ovn-nbctl lsp-add ls0 lsp0-2 -- lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:02 192.168.0.12" done -echo Test failed $fail_count in 10. -AT_CHECK([test $fail_count -le 5]) +check_recompute_counter 0 0 + +# No change, no recompute +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats +check ovn-nbctl --wait=sb sync +check_recompute_counter 0 0 + +CHECK_NO_CHANGE_AFTER_RECOMPUTE OVN_CLEANUP([hv1]) AT_CLEANUP -- 2.30.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
