On Tue, Jun 20, 2023 at 12:48 AM Dumitru Ceara <[email protected]> wrote: > > On 6/20/23 03:49, Han Zhou wrote: > > On Tue, Jun 6, 2023 at 7:57 AM Dumitru Ceara <[email protected]> wrote: > >> > >> We don't need to explicitly add port bindings that were already bound > >> locally. We implicitly get those because we monitor the datapaths > >> they're attached to. > >> > >> When performing an ovn-heater 500-node density-heavy scale test [0], with > >> conditional monitoring enabled, the unreasonably long poll intervals on > >> the Southbound database (the ones that took more than one second) are > >> measured as: > >> > >> --------------------------------------------------------------------- > >> Count Min Max Median Mean 95 percentile > >> --------------------------------------------------------------------- > >> 56.0 1010.0 2664.0 1455.5 1544.9 2163.0 > >> 77.0 1015.0 3460.0 1896.0 1858.2 2907.8 > >> 69.0 1010.0 3118.0 1618.0 1688.1 2582.4 > >> --------------------------------------------------------------------- > >> 202.0 1010.0 3460.0 1610.0 1713.3 2711.5 > >> > >> Compared to the baseline results (also with conditional monitoring > >> enabled): > >> --------------------------------------------------------------------- > >> Count Min Max Median Mean 95 percentile > >> --------------------------------------------------------------------- > >> 141.0 1003.0 18338.0 1721.0 2625.4 7626.0 > >> 151.0 1019.0 80297.0 1808.0 3410.7 9089.0 > >> 165.0 1007.0 50736.0 2354.0 3067.7 7309.8 > >> --------------------------------------------------------------------- > >> 457.0 1003.0 80297.0 2131.0 3044.6 7799.6 > >> > >> We see significant improvement on the server side. This is explained > >> by the fact that now the Southbound database server doesn't need to > >> apply as many conditions as before when filtering individual monitor > >> contents. > >> > > Thanks Dumitru for the great improvement! This is very helpful for the high > > port-density environment. > > Just to make sure I understand the test result correctly, in [0], it shows > > 22500 pods and 500 nodes, so is it 45 pods per node? > > > > Yes, for density-heavy tests (load balancers are also configured) the > pod density is 45 per node. > > >> Note: Sub-ports - OVN switch ports with parent_port set - have to be > >> monitored unconditionally as we cannot efficiently determine their local > >> datapaths without including all local OVS interfaces in the monitor. > >> This, however, should not be a huge issue because the majority of ports > >> are regular VIFs, not sub-ports. > > > > I am not sure if we can make such a conclusion. For the current ovn-k8s or > > environments similar to that, it shouldn't be a problem. > > However, for environments that model pods/containers as sub-ports of the VM > > VIFs, probably most of the majority of the ports would be sub-ports. This > > is what sub-ports are designed for, right? > > My impression was that this was one of the use cases for OpenStack and > that it's only one of the different ways of providing container > connectivity in a given deployment. But I might be wrong. I can remove > this sentence, it makes a lot of assumptions indeed. > > > So, I think this would be a significant change of data monitored for those > > environments. I'd suggest at least we should properly document the > > implication in the documents (such as ovn-monitor-all, and also the > > sub-port related parts). There may also be such users who prefer not > > monitoring all sub-ports (for efficiency of ovn-controller) sacrificing SB > > DB performance (probably they don't have very high port density so the > > conditional monitoring impact is not that big). I am not aware of any such > > users yet, but if they complain, we will have to provide a knob, if no > > better ideas. > > > > I agree, if really needed, we can easily add a knob. > > What do you think of the following incremental? I can fold it in if it > looks good to you.
Thanks Dumitru. The below documentation looks good. In addition, I think we should add some notes in the ovn-nb.xml under the section <group title="Containers"> of Logical_Switch_Port, which is the place where the "sub-port" feature is described. Could you add it as well? Regards, Han > > diff --git a/TODO.rst b/TODO.rst > index 2a5c68ea3f..115cf54e70 100644 > --- a/TODO.rst > +++ b/TODO.rst > @@ -189,3 +189,9 @@ OVN To-do List > * Load Balancer templates > > * Support combining the VIP IP and port into a single template variable. > + > +* ovn-controller conditional monitoring > + > + * Improve sub-ports (with parent_port set) conditional monitoring; these > + are currently unconditionally monitored, even if ovn-monitor-all is > + set to false. > diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml > index f61f430084..0883d8da9b 100644 > --- a/controller/ovn-controller.8.xml > +++ b/controller/ovn-controller.8.xml > @@ -128,6 +128,12 @@ > to <code>true</code> for environments that all workloads need to be > reachable from each other. > </p> > + <p> > + NOTE: for efficiency and scalability in common scenarios > + <code>ovn-controller</code> unconditionally monitors all sub-ports > + (ports with <code>parent_port</code> set) regardless of the > + <code>ovn-monitor-all</code> value. > + </p> > <p> > Default value is <var>false</var>. > </p> > --- > > > Other than this, the patch looks good to me. > > > > Acked-by: Han Zhou <[email protected]> > > > > Thanks for the review! > > Regards, > Dumitru > > >> > >> [0] > > https://github.com/ovn-org/ovn-heater/blob/main/test-scenarios/ocp-500-density-heavy.yml > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
