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.
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
