On Mon, Dec 18, 2023 at 9:08 AM Dumitru Ceara <dce...@redhat.com> wrote: > > On 11/28/23 03:38, num...@ovn.org wrote: > > From: Numan Siddique <num...@ovn.org> > > > > Any changes to northd engine node due to load balancers > > are now handled in 'sync_to_sb_lb' node to sync the changed > > load balancers to SB load balancers. > > > > The logic to sync the SB load balancers is changed a bit and it > > now mimics the SB lflow sync. > > > > Below are the scale testing results done with all the patches applied > > in this series using ovn-heater. The test ran the scenario - > > ocp-500-density-heavy.yml [1]. > > > > The resuts are: > > > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > Min (s) Median (s) 90%ile (s) > > 99%ile (s) Max (s) Mean (s) Total (s) Count > > Failed > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > Iteration Total 0.136883 1.129016 1.192001 > > 1.204167 1.212728 0.665017 83.127099 125 0 > > Namespace.add_ports 0.005216 0.005736 0.007034 > > 0.015486 0.018978 0.006211 0.776373 125 0 > > WorkerNode.bind_port 0.035030 0.046082 0.052469 > > 0.058293 0.060311 0.045973 11.493259 250 0 > > WorkerNode.ping_port 0.005057 0.006727 1.047692 > > 1.069253 1.071336 0.266896 66.724094 250 0 > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > > > The results with the present main [2] are: > > > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > Min (s) Median (s) 90%ile (s) > > 99%ile (s) Max (s) Mean (s) Total (s) Count > > Failed > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > Iteration Total 0.135491 2.223805 3.311270 > > 3.339078 3.345346 1.729172 216.146495 125 0 > > Namespace.add_ports 0.005380 0.005744 0.006819 > > 0.018773 0.020800 0.006292 0.786532 125 0 > > WorkerNode.bind_port 0.034179 0.046055 0.053488 > > 0.058801 0.071043 0.046117 11.529311 250 0 > > WorkerNode.ping_port 0.004956 0.006952 3.086952 > > 3.191743 3.192807 0.791544 197.886026 250 0 > > ------------------------------------------------------------------------------------------------------------------------------------------------------- > > > > [1] - > > https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml > > [2] - 2a12cda890a7("controller, northd: Wait for cleanup before replying to > > exit") > > > > Signed-off-by: Numan Siddique <num...@ovn.org> > > --- > > northd/en-sync-sb.c | 445 +++++++++++++++++++++++++++++++++++++-- > > northd/inc-proc-northd.c | 1 + > > northd/lflow-mgr.c | 196 ++++++----------- > > northd/lflow-mgr.h | 23 +- > > northd/northd.c | 238 --------------------- > > northd/northd.h | 6 - > > tests/ovn-northd.at | 103 +++++---- > > 7 files changed, 585 insertions(+), 427 deletions(-) > > > > Hi Numan, > > The only reason why we need to not ignore SB.LB updates is because we > need to check for duplicates that under normal circumstances do not > exist. They can appear due to "spurious" [0] inserts because the LB > table doesn't have an index defined. > > Would it be less of an effort to just have a periodic task (we do > similar work for mac binding aging) and in that task (I-P node run > function) check for SB load balancer duplicates, remove them and trigger > a full recompute? > > Wouldn't that be less error prone than all the I-P code?
Hi Dumitru, Thanks for the reviews. Do you mean that by adding this periodic task, we can drop this patch ? This patch doesn't do any changes to the existing code which handles the duplicates. It adds I-P to the Load balancer changes to the sb lb sync in the sync_to_sb_lb_northd_handler() function. northd node in its tracked data provides the information about deleted and added/updated load balancers and sync_to_sb_lb_northd_handler() calls sync_changed_lbs() to sync the changed LBs in the SB DB. Before this patch any change to the NB load balancers/lb groups resulted in the sync_to_sb_lb full recompute and this was expensive. Hence added the I-P. Since this patch doesn't change anything related to the duplicates, I think we could still add a periodic task for that (which would be independent to this patch). Thanks Numan > > Thanks, > Dumitru > > [0] https://github.com/ovn-org/ovn/commit/9deb000536e0 > > _______________________________________________ > 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
