| Message ID | 20210304041012.4128938-1-blp@ovn.org |
|---|---|
| Headers | show |
| Series | ovn-northd-ddlog improvements | expand |
On Thu, Mar 4, 2021 at 9:40 AM Ben Pfaff <blp@ovn.org> wrote: > > The first commit here is an improvement to the tests. It is not > ddlog specific: > tests: Improve synchronization and debuggability. > > The next commit is an improvement to ovs-sandbox for > ovn-northd-ddlog: > ovs-sandbox: Make it possible to disable recording ddlog input. > > The next several commits improve the ddlog code from my viewpoint > as someone who likes to read code, without changing its behavior: > ovn-northd-ddlog: Improve type safety for datapath stages. > ovn-northd-ddlog: Use object form of is_some(), drop is_none(). > ovn-northd-ddlog: Make map_get_*() more object-like. > ovn-northd-ddlog: Add general-purpose bitwise library. > ovn-northd-ddlog: Define in_addr, in6_addr, eth_addr in ddlog code. > > These commits are performance improvements. The one in the middle > (come to think of it, it should not be in the middle) modifies > ovn-northd-ddlog.c, the other two modify the ddlog code: > ovn-northd-ddlog: Avoid N*M crossproduct joining switches with > routers. > ovn-northd-ddlog: Apply multiple database updates in single ddlog txn. > ovn-northd-ddlog: Rephrase RouterStaticRoute rule. > > This makes it easier to debug performance: > ovn-northd-ddlog: Add profiling support. > > northd/automake.mk | 4 +- > northd/bitwise.dl | 272 ++++++++++ > northd/bitwise.rs | 133 +++++ > northd/helpers.dl | 22 +- > northd/ipam.dl | 66 ++- > northd/lrouter.dl | 55 +- > northd/lswitch.dl | 26 +- > northd/multicast.dl | 52 +- > northd/ovn-northd-ddlog.c | 155 ++++-- > northd/ovn-northd.8.xml | 36 +- > northd/ovn.dl | 286 +++++------ > northd/ovn.rs | 295 ++++------- > northd/ovn_northd.dl | 1018 +++++++++++++++++-------------------- > tests/ovn.at | 23 +- > tutorial/ovs-sandbox | 7 +- > 15 files changed, 1373 insertions(+), 1077 deletions(-) > create mode 100644 northd/bitwise.dl > create mode 100644 northd/bitwise.rs Thanks for this series. Acked-by: Numan Siddique <numans@ovn.org> Please note that I'm still learning ddlog and haven't understood much of the northd ddlog code. I ran the patches and found a few northd-ddlog tests to be failing when run with -j5. If I run individually they pass. May be the scale improvements in northd-ddlog could help in those tests. I found one particular test failing 100% of the time. Can you please take a look into that. Checking for 1 rows in sb Chassis_Private with name=hv3 nb_cfg=1... found 1 -- 3. *_cfg(*_cfg_timestamp): nb=2(+1117) sb=2(+1107) hv=1(+0) test 2 = 2 ../../tests/ovn-macros.at:327: "$@" test 1614860305430 -gt 1614860304313 ../../tests/ovn-macros.at:327: "$@" test 1614860305487 -gt 1614860304380 ../../tests/ovn-macros.at:327: "$@" test 1614860304343 -gt 0 ../../tests/ovn-macros.at:327: "$@" ovn-controller -vconsole:off --detach --no-chdir --pidfile --log-file Waiting until 1 rows in sb Chassis_Private with name=hv3 nb_cfg=2... ovn-macros.at:378: waiting until test $count = $(count_rows $db:$table $a $b $c $d $e)... ovn-macros.at:378: wait succeeded immediately -- 4. *_cfg(*_cfg_timestamp): nb=2(+0) sb=2(+0) hv=1(-1) hv3_ts=1614860305531 test 1614860305531 = 1614860304342 ../../tests/ovn-macros.at:327: "$@" ../../tests/ovn-macros.at:327: exit code was 1, expected 0 292. ovn.at:23123: 292. ovn -- nb_cfg timestamp -- ovn-northd-ddlog (ovn.at:23123): FAILED (1614860305531) --- Another issue I want to report (which I see happening with the present master). When the sources are configured with "CFLAGS=-g -fsanitize=address", many tests fail with address sanitizer complaining about memory leaks. I have copied below some of the memory logs. Please let me know if you need the complete log files. ----------------- ================================================================= 40. ovn.at:1903: 40. ovn -- 3 HVs, 1 LS, 3 lports/HV -- ovn-northd-ddlog (ovn.at:1903): FAILED (ovs-macros.at:228) ==2426681==ERROR: LeakSanitizer: detected memory leaks Direct leak of 1824 byte(s) in 76 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x4141ff in ddlog_delta_get_table (/home/nusiddiq/workspace_cpp/ovn-org/ovn-for-reviews/ovn/_gcc/northd/ovn-northd-ddlog+0x4141ff) #2 0x410eac in main ../northd/ovn-northd-ddlog.c:1233 #3 0x14fd5d6921e1 in __libc_start_main (/lib64/libc.so.6+0x281e1) Direct leak of 720 byte(s) in 30 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x412a75 in ddlog_transaction_commit_dump_changes (/home/nusiddiq/workspace_cpp/ovn-org/ovn-for-reviews/ovn/_gcc/northd/ovn-northd-ddlog+0x412a75) #2 0x40cf24 in northd_parse_updates ../northd/ovn-northd-ddlog.c:448 #3 0x40d6ac in northd_run ../northd/ovn-northd-ddlog.c:548 #4 0x410e93 in main ../northd/ovn-northd-ddlog.c:1232 #5 0x14fd5d6921e1 in __libc_start_main (/lib64/libc.so.6+0x281e1) Direct leak of 504 byte(s) in 21 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x412a75 in ddlog_transaction_commit_dump_changes (/home/nusiddiq/workspace_cpp/ovn-org/ovn-for-reviews/ovn/_gcc/northd/ovn-northd-ddlog+0x412a75) #2 0x40cf24 in northd_parse_updates ../northd/ovn-northd-ddlog.c:448 #3 0x40d6ac in northd_run ../northd/ovn-northd-ddlog.c:548 #4 0x410e84 in main ../northd/ovn-northd-ddlog.c:1231 #5 0x14fd5d6921e1 in __libc_start_main (/lib64/libc.so.6+0x281e1) Direct leak of 34 byte(s) in 2 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x1a7a4f4 in xmalloc ../lib/util.c:138 #2 0x1a7a4f4 in xvasprintf ../lib/util.c:202 Indirect leak of 32448 byte(s) in 169 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x8dc97e in types::__Rule_OVN_Southbound_Out_Logical_Flow_0::_$u7b$$u7b$closure$u7d$$u7d$::__f::h46d15486e2a82ef3 (/home/nusiddiq/workspace_cpp/ovn-org/ovn-for-reviews/ovn/_gcc/northd/ovn-northd-ddlog+0x8dc97e) Indirect leak of 31640 byte(s) in 113 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x678e25 in alloc::collections::btree::map::BTreeMap$LT$K$C$V$GT$::entry::h7492f993224d96f3 (/home/nusiddiq/workspace_cpp/ovn-org/ovn-for-reviews/ovn/_gcc/northd/ovn-northd-ddlog+0x678e25) Indirect leak of 27968 byte(s) in 76 object(s) allocated from: #0 0x14fd5df453cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf) #1 0x6794b9 in alloc::collections::btree::map::BTreeMap$LT$K$C$V$GT$::insert::he5bd9d63e6aec85d (/home/nusiddiq/workspace_cpp/ovn-org/ovn-for-reviews/ovn/_gcc/northd/ovn-northd-ddlog+0x6794b9) #2 0x410eac in main ../northd/ovn-northd-ddlog.c:1233 #3 0x14fd5d6921e1 in __libc_start_main (/lib64/libc.so.6+0x281e1) .... Thanks Numan ---------------- > > -- > 2.29.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Thu, Mar 04, 2021 at 06:07:42PM +0530, Numan Siddique wrote: > On Thu, Mar 4, 2021 at 9:40 AM Ben Pfaff <blp@ovn.org> wrote: > Thanks for this series. > > Acked-by: Numan Siddique <numans@ovn.org> > > Please note that I'm still learning ddlog and haven't understood much > of the northd ddlog code. > > I ran the patches and found a few northd-ddlog tests to be failing > when run with -j5. If I run individually they pass. > May be the scale improvements in northd-ddlog could help in those tests. > > I found one particular test failing 100% of the time. > > Can you please take a look into that. Thanks for all the comments. I'm looking into this stuff now.
On Thu, Mar 04, 2021 at 10:15:47AM -0800, Ben Pfaff wrote: > On Thu, Mar 04, 2021 at 06:07:42PM +0530, Numan Siddique wrote: > > On Thu, Mar 4, 2021 at 9:40 AM Ben Pfaff <blp@ovn.org> wrote: > > Thanks for this series. > > > > Acked-by: Numan Siddique <numans@ovn.org> > > > > Please note that I'm still learning ddlog and haven't understood much > > of the northd ddlog code. > > > > I ran the patches and found a few northd-ddlog tests to be failing > > when run with -j5. If I run individually they pass. > > May be the scale improvements in northd-ddlog could help in those tests. > > > > I found one particular test failing 100% of the time. > > > > Can you please take a look into that. > > Thanks for all the comments. I'm looking into this stuff now. I pushed the following three commits: * "tests: Improve synchronization and debuggability." * "ovs-sandbox: Make it possible to disable recording ddlog input." * "ovn-northd-ddlog: Add profiling support." I fixed a problem in "ovn-northd-ddlog: Apply multiple database updates in single ddlog txn.", which was leading to the failures you noticed. I'll put the fix in v2.
On Thu, Mar 04, 2021 at 06:07:42PM +0530, Numan Siddique wrote: > On Thu, Mar 4, 2021 at 9:40 AM Ben Pfaff <blp@ovn.org> wrote: > Another issue I want to report (which I see happening with the present master). > When the sources are configured with "CFLAGS=-g -fsanitize=address", > many tests fail with address sanitizer > complaining about memory leaks. I have copied below some of the > memory logs. Please let me know > if you need the complete log files. I sent out two series with memory leak fixes (should have been just one series but I noticed the second leak a bit later): https://mail.openvswitch.org/pipermail/ovs-dev/2021-March/381117.html https://mail.openvswitch.org/pipermail/ovs-dev/2021-March/381119.html
On Tue, Mar 09, 2021 at 01:54:08PM -0800, Ben Pfaff wrote: > On Thu, Mar 04, 2021 at 10:15:47AM -0800, Ben Pfaff wrote: > > On Thu, Mar 04, 2021 at 06:07:42PM +0530, Numan Siddique wrote: > > > On Thu, Mar 4, 2021 at 9:40 AM Ben Pfaff <blp@ovn.org> wrote: > > > Thanks for this series. > > > > > > Acked-by: Numan Siddique <numans@ovn.org> > > > > > > Please note that I'm still learning ddlog and haven't understood much > > > of the northd ddlog code. > > > > > > I ran the patches and found a few northd-ddlog tests to be failing > > > when run with -j5. If I run individually they pass. > > > May be the scale improvements in northd-ddlog could help in those tests. > > > > > > I found one particular test failing 100% of the time. > > > > > > Can you please take a look into that. > > > > Thanks for all the comments. I'm looking into this stuff now. > > I pushed the following three commits: > > * "tests: Improve synchronization and debuggability." > * "ovs-sandbox: Make it possible to disable recording ddlog input." > * "ovn-northd-ddlog: Add profiling support." > > I fixed a problem in "ovn-northd-ddlog: Apply multiple database updates > in single ddlog txn.", which was leading to the failures you noticed. > I'll put the fix in v2. The fix is really solid in my testing, so I decided to just push this series given that I have a new series of improvements that I want to post and making it a much longer series is going to make review difficult.
The first commit here is an improvement to the tests. It is not ddlog specific: tests: Improve synchronization and debuggability. The next commit is an improvement to ovs-sandbox for ovn-northd-ddlog: ovs-sandbox: Make it possible to disable recording ddlog input. The next several commits improve the ddlog code from my viewpoint as someone who likes to read code, without changing its behavior: ovn-northd-ddlog: Improve type safety for datapath stages. ovn-northd-ddlog: Use object form of is_some(), drop is_none(). ovn-northd-ddlog: Make map_get_*() more object-like. ovn-northd-ddlog: Add general-purpose bitwise library. ovn-northd-ddlog: Define in_addr, in6_addr, eth_addr in ddlog code. These commits are performance improvements. The one in the middle (come to think of it, it should not be in the middle) modifies ovn-northd-ddlog.c, the other two modify the ddlog code: ovn-northd-ddlog: Avoid N*M crossproduct joining switches with routers. ovn-northd-ddlog: Apply multiple database updates in single ddlog txn. ovn-northd-ddlog: Rephrase RouterStaticRoute rule. This makes it easier to debug performance: ovn-northd-ddlog: Add profiling support. northd/automake.mk | 4 +- northd/bitwise.dl | 272 ++++++++++ northd/bitwise.rs | 133 +++++ northd/helpers.dl | 22 +- northd/ipam.dl | 66 ++- northd/lrouter.dl | 55 +- northd/lswitch.dl | 26 +- northd/multicast.dl | 52 +- northd/ovn-northd-ddlog.c | 155 ++++-- northd/ovn-northd.8.xml | 36 +- northd/ovn.dl | 286 +++++------ northd/ovn.rs | 295 ++++------- northd/ovn_northd.dl | 1018 +++++++++++++++++-------------------- tests/ovn.at | 23 +- tutorial/ovs-sandbox | 7 +- 15 files changed, 1373 insertions(+), 1077 deletions(-) create mode 100644 northd/bitwise.dl create mode 100644 northd/bitwise.rs