On 6/20/23 21:01, Han Zhou wrote: > 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? >
To avoid an "incremental on top of another incremental" I posted v2. I also added a NEWS item as this is a user-visible change. Please let me know what you think. https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
