[ovs-git] [ovn-org/ovn] d7bfdd: ovn-northd-ddlog: Fix two memory leaks.
Ben Pfaff
noreply at github.com
Wed Apr 7 16:48:17 UTC 2021
Branch: refs/heads/ddlog26
Home: https://github.com/ovn-org/ovn
Commit: d7bfdd7d31d39e10e02485a8d188d3a44d8b6129
https://github.com/ovn-org/ovn/commit/d7bfdd7d31d39e10e02485a8d188d3a44d8b6129
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/ovn-northd-ddlog.c
Log Message:
-----------
ovn-northd-ddlog: Fix two memory leaks.
I get a clean report from Address Sanitizer now.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: f98bf02b14c1674bedf6768de9ccb69e774257c5
https://github.com/ovn-org/ovn/commit/f98bf02b14c1674bedf6768de9ccb69e774257c5
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M utilities/ovn-nbctl.c
Log Message:
-----------
ovn-nbctl: Fix memory leak in client mode.
This isn't notable, since this commit frees it just before exiting, but
it cleans up the Address Sanitizer report.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: d643444b09bb5cd1e2d50c1dce514cf9c2a80c4e
https://github.com/ovn-org/ovn/commit/d643444b09bb5cd1e2d50c1dce514cf9c2a80c4e
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M utilities/ovn-nbctl.8.xml
Log Message:
-----------
ovn-nbctl: Improve manpage.
This rearranges the manpage into a more logical order, documents some
options that weren't documented, adds some sections such as
Environment and Exit Status that a manpage should have, puts the
headings at reasonable levels instead of all at the top level, and adds
a little more explanatory text in a few places.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 43ac7e06f52d439827210ca7324cceb94680a7c0
https://github.com/ovn-org/ovn/commit/43ac7e06f52d439827210ca7324cceb94680a7c0
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M utilities/ovn-nbctl.8.xml
Log Message:
-----------
ovn-nbctl: Recommend ovn-appctl instead of ovs-appctl.
They do exactly the same thing but ovn-appctl has more ovn in its name.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Suggested-by: Mark Michelson <mmichels at redhat.com>
Commit: 37a020072d51dab6b8ec0bd7a5016349df46ace1
https://github.com/ovn-org/ovn/commit/37a020072d51dab6b8ec0bd7a5016349df46ace1
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M NEWS
M utilities/ovn-nbctl.8.xml
Log Message:
-----------
ovn-nbctl: Daemon mode is no longer experimental.
Mark says that it was heavily used by ovn-kubernetes for years without
any issues.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Suggested-by: Mark Michelson <mmichels at redhat.com>
Commit: 663d94f1b48a2d184b5b04d41f33249a528afb26
https://github.com/ovn-org/ovn/commit/663d94f1b48a2d184b5b04d41f33249a528afb26
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M utilities/automake.mk
A utilities/ovn-dbctl.c
A utilities/ovn-dbctl.h
M utilities/ovn-nbctl.c
Log Message:
-----------
ovn-nbctl: Refactor into infrastructure and northbound details.
In an upcoming commit, this will allow adding daemon mode to ovn-sbctl
without having a lot of duplicated code.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: a9996428ae39a1f53a5fd374c8baedc2192a53d5
https://github.com/ovn-org/ovn/commit/a9996428ae39a1f53a5fd374c8baedc2192a53d5
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M utilities/ovn-dbctl.c
Log Message:
-----------
ovn-dbctl: Fix memory leak in client mode.
This isn't notable, since this commit frees it just before exiting, but
it cleans up the Address Sanitizer report.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: e72c5ecb083e8562ee2bbd84599e0a5c71601556
https://github.com/ovn-org/ovn/commit/e72c5ecb083e8562ee2bbd84599e0a5c71601556
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M NEWS
M manpages.mk
M tests/ovn-sbctl.at
M utilities/automake.mk
M utilities/ovn-dbctl.c
M utilities/ovn-dbctl.h
M utilities/ovn-nbctl.c
R utilities/ovn-sbctl.8.in
A utilities/ovn-sbctl.8.xml
M utilities/ovn-sbctl.c
Log Message:
-----------
ovn-sbctl: Add daemon support.
Also rewrite the manpage and convert it to XML for consistency with
ovn-nbctl, and add tests.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: ae63e2c6dfed883b7eac2daa5a9e3def04928d88
https://github.com/ovn-org/ovn/commit/ae63e2c6dfed883b7eac2daa5a9e3def04928d88
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M tests/ovn.at
Log Message:
-----------
tests: Miscellaneous debuggability improvements.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 575f5f316a04a637c50a93079bbd642ff1f85cc7
https://github.com/ovn-org/ovn/commit/575f5f316a04a637c50a93079bbd642ff1f85cc7
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M NEWS
M northd/lrouter.dl
M northd/ovn-northd-ddlog.c
Log Message:
-----------
ovn-northd-ddlog: Upgrade to ddlog 0.38.
Upcoming commits will use a new --intern-table option of ovsdb2ddlog,
so we need to upgrade to the version of ddlog that has that feature.
To do so, we need to adapt the code to language changes in ddlog. This
commit does that for a change in 0.37 in which, when iterating over a
`Group` in a for-loop, the iterator returns `(value, weight)` tuples.
This also adapts ovn-northd-ddlog.c to a slightly updated C API.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 3b86b40567cc7d95f850e165264ca3d3dc2d3e59
https://github.com/ovn-org/ovn/commit/3b86b40567cc7d95f850e165264ca3d3dc2d3e59
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Preserve NB_Global more carefully.
Dumitru reported in #openvswitch that ovn-northd-ddlog can discard the
setting of NB_Global.options:use_logical_dp_groups at startup. I think
that this must be because it seems possible that at startup some of the
relations in the Out_NB_Global rule aren't populated yet, and yet
there is still a row in nb::NB_Global, so that neither rule for
Out_NB_Global matches and therefore ovn-northd-ddlog deletes the row.
This commit changes the structure of how ovn-northd-ddlog generates
Out_NB_Global to ensure that, if there's an input row, it always
generates exactly one output row. This should be more robust than the
previous version regardless of whether it fixes the exact problem
that Dumitru observed (which I did not try to reproduce).
Reported-by: Dumitru Ceara <dceara at redhat.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 7f925cfe9d40754c0a607c4800ad078a76ac8a64
https://github.com/ovn-org/ovn/commit/7f925cfe9d40754c0a607c4800ad078a76ac8a64
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/lrouter.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Remove `lr` field from `Router`.
`relation Router` stores the internal representation of a logical
router, consisting of values from the `nb::Logical_Router` table
augmented with some additional fields. We used to do this by
copying the entire `Logical_Router` record inside `Router`. This
proved highly inefficient in scenarios where the set of router ports
changes frequently. Every such change modifies the `ports` array
inside `Logical_Router`, which triggers an update of the `Router`
object, which can cause a bunch of rules to update their outputs. This
recomputation is unnecessary as none of these rules look at the `ports`
field (`ports` is a slightly backwards way to maintain the relationship
between ports and routers by storing the array of ports in the router
instead of having each port point to the router).
As a workaround, we no longer store the entire `Logical_Router` object
in the `Router` table, and instead only copy its relevant fields.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 28c54fa228bcfa6990ca34387d580776d4bf00e5
https://github.com/ovn-org/ovn/commit/28c54fa228bcfa6990ca34387d580776d4bf00e5
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/lrouter.dl
M northd/multicast.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Intern the `Router` table.
This is the first in a series of commits that will replace the use of
the DDlog's `Ref<>` type with `Intern<>` throughout the OVN code base.
`Ref` and `Intern` are the two forms of smart pointers supported by
DDlog at the moment. `Ref` is a reference counted pointer. Copying
a `Ref<>` simply increments its reference count. `Intern<>` is an
interned object reference. It guarantees that there exists exactly
one copy of each unique interned value. Interned objects are slightly
more expensive to create, but they have several important advantages:
(1) they save memory by deduplicating identical values, (2) they allow
by-pointer comparisons, and (3) they avoid unnecessary recomputations
in some scenarios. See DDlog docs [1], [2] for more detail.
In this commit we change the type of records in the `Router` table from
`Ref<Router>` to `Intern<Router>`. This reduces the amount of churn
and speeds up northd significantly in scenarios where the set of router
ports changes frequently, which triggers updates to
`nb::Logical_Router`, which in turn updates corresponding records
in the `Router` table. Interning guarantees that these updates are
no-ops and do not trigger any other rules.
[1] https://github.com/vmware/differential-datalog/blob/master/doc/tutorial/tutorial.md#reference-type-ref
[2] https://github.com/vmware/differential-datalog/blob/master/doc/tutorial/tutorial.md#interned-values-intern-istring
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: a840fe0cd1abd1ebbb595904e7dcd483986d3277
https://github.com/ovn-org/ovn/commit/a840fe0cd1abd1ebbb595904e7dcd483986d3277
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Workaround for slow group_by.
This patch is a workaround for a performance issue in the DDlog
compiler. The issue will hopefully be resolved in a future version of
DDlog, but for now we need this and possibly a few other similar fixes.
Here is one affected rule:
```
sb::Out_Port_Group(._uuid = hash128(sb_name), .name = sb_name, .ports = port_names) :-
nb::Port_Group(._uuid = _uuid, .name = nb_name, .ports = pg_ports),
var port_uuid = FlatMap(pg_ports),
&SwitchPort(.lsp = lsp at nb::Logical_Switch_Port{._uuid = port_uuid,
.name = port_name},
.sw = &Switch{.ls = nb::Logical_Switch{._uuid = ls_uuid}}),
TunKeyAllocation(.datapath = ls_uuid, .tunkey = tunkey),
var sb_name = "${tunkey}_${nb_name}",
var port_names = port_name.group_by((_uuid, sb_name)).to_set().
```
The first literal in the body of the rule binds variable `pg_ports` to
the array of ports in the port group. This is a potentially large
array that immediately gets flattened by the `FlatMap` operator.
Since the `pg_ports` variable is not used in the remainder of the rule,
DDlog normally would not propagate it through the rest of the rule.
Unfortunately, due to a subtle semantic quirk, the behavior is different
when there is a `group_by` operator further down in the rule, in which
case unused variables are still propagated through the rule, which
involves expensive copies.
The workaround I implemented factors the first two terms in the
rule into a separate `PortGroupPort` relation, so that the ports array
no longer occurs in the new version of the rule:
```
sb::Out_Port_Group(._uuid = hash128(sb_name), .name = sb_name, .ports = port_names) :-
PortGroupPort(.pg_uuid = _uuid, .pg_name = nb_name, .port = port_uuid),
&SwitchPort(.lsp = lsp at nb::Logical_Switch_Port{._uuid = port_uuid,
.name = port_name},
.sw = &Switch{.ls = nb::Logical_Switch{._uuid = ls_uuid}}),
TunKeyAllocation(.datapath = ls_uuid, .tunkey = tunkey),
var sb_name = "${tunkey}_${nb_name}",
var port_names = port_name.group_by((_uuid, sb_name)).to_set().
```
Again, benchmarking is likely to reveal more instances of this. A
proper fix will require a change to the DDlog compiler.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 356e111e7893c4bdfc97b606a0e88b2e603c157f
https://github.com/ovn-org/ovn/commit/356e111e7893c4bdfc97b606a0e88b2e603c157f
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/lswitch.dl
M northd/multicast.dl
Log Message:
-----------
ovn-northd-ddlog: Intern the Switch table.
Change the type of record in the `Switch` table from `Ref<Switch>` to
`Intern<Switch>`.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: bced09355d1581781c0e1b9cb59ba686ee0f004a
https://github.com/ovn-org/ovn/commit/bced09355d1581781c0e1b9cb59ba686ee0f004a
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/ipam.dl
M northd/lswitch.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Remove `ls` field from `Switch`.
This commit is analogous to 076749c99, but switches instead of routers.
`relation Switch` stores the internal representation of a logical
switch, consisting of values from the `nb::Logical_Switch` table
augmented with some additional fields. We used to do this by
copying the entire `Logical_Switch` record inside `Switch`. This
proved highly inefficient in scenarios where some of the entities that
`Logical_Switch` references (logicl switch ports, ACLs, or QoS rules)
change frequently. Every such change modifies the `Logical_Switch`
record, which triggers an update of the `Switch` object, which can cause
a bunch of rules to update their outputs.
As a workaround, we no longer store the entire `Logical_Switch` object
in the `Switch` table, and instead only copy its relevant fields.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 001526149d13e497faa850541251f06026b396a9
https://github.com/ovn-org/ovn/commit/001526149d13e497faa850541251f06026b396a9
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/ipam.dl
M northd/lswitch.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Intern the SwitchPort table.
Change the type of record in the `SwitchPort` table from
`Ref<SwitchPort>` to `Intern<SwitchPort>`.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: dc7070e990ddd176d9035feaf812c3a7bb6b5380
https://github.com/ovn-org/ovn/commit/dc7070e990ddd176d9035feaf812c3a7bb6b5380
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/lrouter.dl
M northd/lswitch.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Intern the RouterPort table.
Change the type of record in the `RouterPort` table from
`Ref<RouterPort>` to `Intern<RouterPort>`.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 3f19b1bd3460fb566c7ffb0c0e538785f3cffa37
https://github.com/ovn-org/ovn/commit/3f19b1bd3460fb566c7ffb0c0e538785f3cffa37
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/lswitch.dl
Log Message:
-----------
ovn-northd-ddlog: Remove unused function.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: a9d6fb5ad3fe9857cec9454a866d2035a937e154
https://github.com/ovn-org/ovn/commit/a9d6fb5ad3fe9857cec9454a866d2035a937e154
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/helpers.dl
M northd/lrouter.dl
M northd/lswitch.dl
M northd/multicast.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Eliminate remaining Ref's.
Change all remaining occurrences of `Ref<T>` to `Intern<T>` throughout
the DDlog code base.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 1905506dc4d2e52f93d0085210e2a17a6c77ea1e
https://github.com/ovn-org/ovn/commit/1905506dc4d2e52f93d0085210e2a17a6c77ea1e
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/ipam.dl
M northd/multicast.dl
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Eliminate redundant dereferences.
We eliminate an anti-pattern in the use of smart pointers that occurred
throughout the DDlog code.
Consider relation `A` that contains field `x` wrapped in a DDlog smart
pointer (`Intern<>` or `Ref<>`):
```
relation A(x: Intern<T>, ...)
```
Here `T` might be a complex type with dynamically allocated fields like
vectors and maps, etc. Here is how _not_ to use this relation in a
rule:
```
Rel(...) :- A(.x = &v),
B(v.field1),
C(v.field2).
```
The `&v` syntax here extracts the inner value that the smart pointer
points to and binds it to variable `v`. Thus, the type of `v` is `T`
and we thread the value of `T` through the entire rule, which requires
creating two more copies of it (types not wrapped in smart pointers are
copied by value). This is a waste of memory and CPU and is completely
unnecessary, as we can instead bind `v` to the value of the smart
pointer, so it can be copied efficiently:
```
Rel(...) :- A(.x = v), // type of `v` is `Intern<T>`.
B(v.field1),
C(v.field2).
```
The inefficient usage if a leftover from the days when DDlog had some
awkward restrictions on the use of smart pointers.
Note that `&` is still useful and does not incur any overhead when used
to deconstruct an object wrapped in a smart pointer and refer to its
fields, e.g.:
```
Rel(...) :- // Bind `f1` and `f2` to `x.field1` and `x.field2`;
// filter out records where `field3` is `false`.
A(.x = &T{.field1 = f1, .field2 = f2, .field3 = true}),
B(f1),
C(f2).
```
On top of this, the `@` operator can be used to simultaneously bind the
entire value stored in `x` and its individual fields.
```
Rel(...) :- // Bind `v` to the value of field `x` (`v: Intern<T>`);
// bind `f2` to `x.field2`; filter on the value of `field3`.
A(.x = v @ &T{.field2 = f2, .field3 = true}),
B(v.field1),
C(f2).
```
The `&` in this rule is used to deconstruct the value inside the smart
pointer; however since the binding operator `@` preceeds `&`, we bind
`v` to the value of the smart pointer and not the type that it wraps.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 4d9360ff1b9e64246424735d3e80e5f0b434d5d0
https://github.com/ovn-org/ovn/commit/4d9360ff1b9e64246424735d3e80e5f0b434d5d0
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/helpers.dl
M northd/lrouter.dl
M northd/lswitch.dl
M northd/ovn-nb.dlopts
M northd/ovn-sb.dlopts
M northd/ovn_northd.dl
M northd/ovsdb2ddlog2c
Log Message:
-----------
ovn-northd-ddlog: Intern selected input relations.
DDlog 0.38.0 adds the `--intern-table` CLI flag to the `ovsdb2ddlog`
compiler to declare input tables coming from OVSDB as `Intern<...>`.
This is useful for tables whose records are copied around as a whole and
can therefore benefit from interning performance- and memory-wise. In
the past we had to create separate tables in `helpers.dl` and copy
records from the original input table to them while wrapping them in
`Intern<>`. With this change, we avoid the extra copy and intern
records as we ingest them for selected tables.
We use the `--intern-table` flag to eliminate all intermediate tables in
`helpers.dl`.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 6b643c6a8a7e1668039aa31bdb7d19d9a8aa96bc
https://github.com/ovn-org/ovn/commit/6b643c6a8a7e1668039aa31bdb7d19d9a8aa96bc
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/helpers.dl
M northd/lrouter.dl
M northd/lswitch.dl
M northd/multicast.dl
M northd/ovn-nb.dlopts
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Intern nb::Logical_Router_Port.
Use the `--intern-table` switch to intern `Logical_Router_Port` records,
so that they can be copied and compared efficiently by pointer.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 8068426c46ade5835ae81d33276a657fdd981bd6
https://github.com/ovn-org/ovn/commit/8068426c46ade5835ae81d33276a657fdd981bd6
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/helpers.dl
M northd/lrouter.dl
M northd/lswitch.dl
M northd/multicast.dl
M northd/ovn-nb.dlopts
M northd/ovn_northd.dl
Log Message:
-----------
ovn-northd-ddlog: Intern nb::Logical_Switch_Port.
Use the `--intern-table` switch to intern `Logical_Switch_Port` records,
so that they can be copied and compared efficiently by pointer.
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: 2a85ff44f49faea9c65f6dfdd4f9f12ecb9af482
https://github.com/ovn-org/ovn/commit/2a85ff44f49faea9c65f6dfdd4f9f12ecb9af482
Author: Leonid Ryzhyk <lryzhyk at vmware.com>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M northd/lrouter.dl
Log Message:
-----------
ovn-northd-ddlog: Remove Router.static_routes.
This is another instance of a performance bug when a change to a
router object forces a cascade of changes to relations that reference
the object. This time around the problem was caused by the
`Router.static_routes` field, which is copied from
`nb::Logical_Router`. Luckily, this field was only used in one rule
and was easy to remove.
Here is how we diagnosed the issue (this may be useful for posterity):
- We started with a benchmark that executed several hundreds of similar
transactions (in this case, these transactions were adding new router
ports). We recorded execution of the benchmark in a DDlog command
file (replay.txt) and added `timestamp;` commands after each
transaction in the file.
- Run `make NORTHD_CLI=1` to generate the ovn_northd_cli executable and
use it to execute the command file:
```
./ovn_northd_ddlog/target/release/ovn_northd_cli -w 1 < replay.txt > replay.dump
```
- Extract only the timestamps from replay.dump and plot differences
between successive timestamps (i.e., individual transaction times).
I use gnumeric. It would be nice to have some automation for this
in the future. We observe that one of the transactions in the
benchmark loop slows down linearly as the size of the network
topology grows:
https://gist.github.com/ryzhyk/16a5607b280ed9cd09b176d6816cb4f0
Clearly some of the rules in the program are getting more expensive
as the number of ports goes up. Another interesting observation is
that the size of the delta output at each iteration of the benchmark
remains constant (the delta mostly consists of new network flows).
This suggests that whatever extra work DDlog is doing at each
iteration might be redundant.
- To identify where the wasted work happens, we re-compile the program
passing the `--output-internal-relations` flag to DDlog, which tells it
to dump changes to all intermediate relations, not just output
relation. We replay the trace again. We locate the expensive
transaction in the log and compare its output from one of the first
iterations vs one from the end of the log. We now see a whole bunch of
intermediate relations that only had a few modified records in the
first transaction versus hundreds in the second one. We further
observe that all of these changes simply update the `static_routes`
field (as explained above).
- After removing the field, we observe that changes to intermediate
relations no longer grow with the topology, and transaction timing
increases much more slowly:
https://gist.github.com/ryzhyk/d02784b9088d82f8549ea1b2ebdf095e
Signed-off-by: Leonid Ryzhyk <lryzhyk at vmware.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Commit: bc326e2b03b4a4051ad974d0adb8f3dd58a3e807
https://github.com/ovn-org/ovn/commit/bc326e2b03b4a4051ad974d0adb8f3dd58a3e807
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-01 (Thu, 01 Apr 2021)
Changed paths:
M tutorial/automake.mk
A tutorial/northd_ddlog_test.sh
Log Message:
-----------
tutorial: Add benchmarking test script to run within sandbox.
This is originally from Numan Siddique. I have adapted it a bit
to run faster by using the ovn-nbctl and ovn-sbctl daemons and
combining multiple calls into just one.
I'm uncertain whether to actually commit this; I need a sign-off
from Numan to do so.
Signed-off-by: Ben Pfaff <blp at ovn.org>
CC: Numan Siddique <numans at ovn.org>
Commit: 43c6dc63d5de2f292bd0fc5243f8e743d007a7df
https://github.com/ovn-org/ovn/commit/43c6dc63d5de2f292bd0fc5243f8e743d007a7df
Author: Ben Pfaff <blp at ovn.org>
Date: 2021-04-06 (Tue, 06 Apr 2021)
Changed paths:
M utilities/ovn-dbctl.c
M utilities/ovn-nbctl.c
Log Message:
-----------
ovn-nbctl: Don't replicate entire database unnecessarily.
ovn-nbctl was retrieving a replica of the entire northbound database
regardless of what it was asked to do. This was unnecessary, but it
was not written to be smart enough to just retrieve what it needed.
This commit fixes the problem. It makes "ovn-nbctl init", for example,
run instantly on a big database, whereas before it could take
arbitrarily long.
Signed-off-by: Ben Pfaff <blp at ovn.org>
Compare: https://github.com/ovn-org/ovn/compare/d7bfdd7d31d3%5E...43c6dc63d5de
More information about the git
mailing list