| Message ID | 20210903122148.826196-1-mark.d.gray@redhat.com |
|---|---|
| Headers | show |
| Series | northd: Split northd and northd incremental processing framework | expand |
Hi, Dumitru mentioned to me that the impact of change tracking may affect the performance relative to the current code. I did some quick tests with a large NBDB and checked northd timings. The results are below. It doesn't seem to have a significant impact but the data is a bit noisy. Without this patch: ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test1 && ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Time spent on processing nb_cfg 2: ovn-northd delay before processing: 7000ms ovn-northd completion: 14935ms Statistics for 'ovnnb_db_run' Total samples: 5 Maximum: 7915 msec Minimum: 0 msec 95th percentile: 0.000000 msec Short term average: 6732.375000 msec Long term average: 7831.435378 msec ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test2 && ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Time spent on processing nb_cfg 3: ovn-northd delay before processing: 6645ms ovn-northd completion: 14365ms Statistics for 'ovnnb_db_run' Total samples: 5 Maximum: 7600 msec Minimum: 0 msec 95th percentile: 0.000000 msec Short term average: 6618.250000 msec Long term average: 7460.322269 msec ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test3 && ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Time spent on processing nb_cfg 4: ovn-northd delay before processing: 6670ms ovn-northd completion: 14362ms Statistics for 'ovnnb_db_run' Total samples: 5 Maximum: 7568 msec Minimum: 0 msec 95th percentile: 0.000000 msec Short term average: 6615.750000 msec Long term average: 7476.213026 msec With this patch: ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test1 && ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Time spent on processing nb_cfg 2: ovn-northd delay before processing: 112ms ovn-northd completion: 7986ms Statistics for 'ovnnb_db_run' Total samples: 3 Maximum: 7732 msec Minimum: 7535 msec 95th percentile: 7535.000000 msec Short term average: 7620.500000 msec Long term average: 7729.515200 msec ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test2 && ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Time spent on processing nb_cfg 3: ovn-northd delay before processing: 118ms ovn-northd completion: 7826ms Statistics for 'ovnnb_db_run' Total samples: 3 Maximum: 7564 msec Minimum: 7562 msec 95th percentile: 7562.000000 msec Short term average: 7563.250000 msec Long term average: 7563.000100 msec ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test3 && ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Time spent on processing nb_cfg 4: ovn-northd delay before processing: 109ms ovn-northd completion: 7880ms Statistics for 'ovnnb_db_run' Total samples: 3 Maximum: 7767 msec Minimum: 7555 msec 95th percentile: 7555.000000 msec Short term average: 7623.000000 msec Long term average: 7615.904800 msec On 03/09/2021 13:21, Mark Gray wrote: > Part 1: > The first two comits deal with reorganising the northd code base a little. > Please note that this commit mainly involves moving code around with minimal > code changes. However, due to tight coupling between ovn-northd.c and northd.c, > some minor changes were needed. For reference, and to help reviews, please > examine the following at a minimum: > > * Configuration of the probe interval in northd.c (ovsdb_idl_set_probe_interval()) > * Passing of "use_parallel_build" and "lflow_locks" from ovn-northd.c and > northd.c. > * Update of "struct northd_context": additon of fields and move to h file. > > The commits were (hopefully) structured in a way to make the review easier. As > this change touches all of ovn-northd, any change to "master" will make a rebase > necessary and probably difficult. Therefore, if the general ideas is OK, then > it would be great if this series could be expedited to prevent many rebases! > > Part 2: > The remaining commits add the inc-proc-eng framework to northd. This does *not* > add any incremental processing of northd processing at this stage but provides the > framework. Even in this base configuration, we see an advantage as northd no longer > processes the databases if it has been woken up only to handle, for example, a > unixctl command. This can be seen below: > > $ ovn-appctl -t ovn-northd stopwatch/reset > $ for i in {1..10}; do ovn-appctl -t ovn-northd stopwatch/show >/dev/null; done > $ ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Statistics for 'ovnnb_db_run' > Total samples: 0 > Maximum: 0 msec > Minimum: 0 msec > 95th percentile: 0.000000 msec > Short term average: 0.000000 msec > Long term average: 0.000000 msec > > Hopefully this starting point will allow others to discuss or contribute changes to > incrementally process some aspects of northd. We can also decide if it is worth > progressing with "Part 2" if we think there are potential benefits until we start > seeing the benefits of DDlog. > > Thanks > > --- > v2: Rebase > v3: Rebase. Fixed compile-time error > v4: Add additional commits which add framework for incremental processing in northd > > Mark Gray (7): > ovn-northd: Rename ovn-northd.c to northd.c > northd: Split northd.c > inc-proc-eng: Allow definition of engine_node with global scope > northd: Introduce incremental processing for northd > northd: Add n_nat_entries field to 'struct ovn_datapath' > northd: Add runtime node > northd: Add functionality to runtime node > > Documentation/tutorials/ovn-openstack.rst | 154 +- > controller/ovn-controller.c | 2 +- > lib/inc-proc-eng.h | 24 +- > northd/automake.mk | 8 + > northd/en-northd.c | 52 + > northd/en-northd.h | 17 + > northd/en-runtime.c | 66 + > northd/en-runtime.h | 25 + > northd/inc-proc-northd.c | 258 + > northd/inc-proc-northd.h | 15 + > northd/lrouter.dl | 2 +- > northd/northd.c | 14568 +++++++++++++++++++ > northd/northd.h | 45 + > northd/ovn-northd.c | 15084 +------------------- > northd/ovn.rs | 2 +- > northd/ovn_northd.dl | 2 +- > tests/ovn-northd.at | 2 +- > 17 files changed, 15454 insertions(+), 14872 deletions(-) > create mode 100644 northd/en-northd.c > create mode 100644 northd/en-northd.h > create mode 100644 northd/en-runtime.c > create mode 100644 northd/en-runtime.h > create mode 100644 northd/inc-proc-northd.c > create mode 100644 northd/inc-proc-northd.h > create mode 100644 northd/northd.c > create mode 100644 northd/northd.h >
Hi Mark, thanks for doing this. I think what you've shown here is actually more significant than you're giving yourself credit for. The biggest thing this has proven is that it eliminates unnecessary DB runs when processing CLI commands. As you showed, the delay before processing was nearly entirely eliminated, meaning that your commands were handled in half the time. And the number of samples from the stopwatch went from 5 to 3, indicating fewer DB runs were executed. The only oddity here that I have no explanation for is why the stopwatch output pre-patch has 0 for both the minimum and the 95th percentile. It seems impossible that a DB run would ever actually take 0 milliseconds to complete. With the P95 being 0, it also indicates that 4 of the 5 samples collected were 0. That's weird... Overall, this is a welcome improvement if you ask me. On 9/15/21 12:52 PM, Mark Gray wrote: > Hi, > > Dumitru mentioned to me that the impact of change tracking may affect > the performance relative to the current code. I did some quick tests > with a large NBDB and checked northd timings. The results are below. It > doesn't seem to have a significant impact but the data is a bit noisy. > > Without this patch: > > ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl > --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test1 && > ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Time spent on processing nb_cfg 2: > ovn-northd delay before processing: 7000ms > ovn-northd completion: 14935ms > Statistics for 'ovnnb_db_run' > Total samples: 5 > Maximum: 7915 msec > Minimum: 0 msec > 95th percentile: 0.000000 msec > Short term average: 6732.375000 msec > Long term average: 7831.435378 msec > > ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl > --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test2 && > ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Time spent on processing nb_cfg 3: > ovn-northd delay before processing: 6645ms > ovn-northd completion: 14365ms > Statistics for 'ovnnb_db_run' > Total samples: 5 > Maximum: 7600 msec > Minimum: 0 msec > 95th percentile: 0.000000 msec > Short term average: 6618.250000 msec > Long term average: 7460.322269 msec > > ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl > --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test3 && > ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Time spent on processing nb_cfg 4: > ovn-northd delay before processing: 6670ms > ovn-northd completion: 14362ms > Statistics for 'ovnnb_db_run' > Total samples: 5 > Maximum: 7568 msec > Minimum: 0 msec > 95th percentile: 0.000000 msec > Short term average: 6615.750000 msec > Long term average: 7476.213026 msec > > > With this patch: > > ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl > --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test1 && > ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Time spent on processing nb_cfg 2: > ovn-northd delay before processing: 112ms > ovn-northd completion: 7986ms > Statistics for 'ovnnb_db_run' > Total samples: 3 > Maximum: 7732 msec > Minimum: 7535 msec > 95th percentile: 7535.000000 msec > Short term average: 7620.500000 msec > Long term average: 7729.515200 msec > > ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl > --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test2 && > ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Time spent on processing nb_cfg 3: > ovn-northd delay before processing: 118ms > ovn-northd completion: 7826ms > Statistics for 'ovnnb_db_run' > Total samples: 3 > Maximum: 7564 msec > Minimum: 7562 msec > 95th percentile: 7562.000000 msec > Short term average: 7563.250000 msec > Long term average: 7563.000100 msec > > ovn-appctl -t ovn-northd stopwatch/reset ovnnb_db_run && ovn-nbctl > --print-wait-time --wait=sb lsp-add lswitch-ovn-scale-77 test3 && > ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run > Time spent on processing nb_cfg 4: > ovn-northd delay before processing: 109ms > ovn-northd completion: 7880ms > Statistics for 'ovnnb_db_run' > Total samples: 3 > Maximum: 7767 msec > Minimum: 7555 msec > 95th percentile: 7555.000000 msec > Short term average: 7623.000000 msec > Long term average: 7615.904800 msec > > > > On 03/09/2021 13:21, Mark Gray wrote: >> Part 1: >> The first two comits deal with reorganising the northd code base a little. >> Please note that this commit mainly involves moving code around with minimal >> code changes. However, due to tight coupling between ovn-northd.c and northd.c, >> some minor changes were needed. For reference, and to help reviews, please >> examine the following at a minimum: >> >> * Configuration of the probe interval in northd.c (ovsdb_idl_set_probe_interval()) >> * Passing of "use_parallel_build" and "lflow_locks" from ovn-northd.c and >> northd.c. >> * Update of "struct northd_context": additon of fields and move to h file. >> >> The commits were (hopefully) structured in a way to make the review easier. As >> this change touches all of ovn-northd, any change to "master" will make a rebase >> necessary and probably difficult. Therefore, if the general ideas is OK, then >> it would be great if this series could be expedited to prevent many rebases! >> >> Part 2: >> The remaining commits add the inc-proc-eng framework to northd. This does *not* >> add any incremental processing of northd processing at this stage but provides the >> framework. Even in this base configuration, we see an advantage as northd no longer >> processes the databases if it has been woken up only to handle, for example, a >> unixctl command. This can be seen below: >> >> $ ovn-appctl -t ovn-northd stopwatch/reset >> $ for i in {1..10}; do ovn-appctl -t ovn-northd stopwatch/show >/dev/null; done >> $ ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run >> Statistics for 'ovnnb_db_run' >> Total samples: 0 >> Maximum: 0 msec >> Minimum: 0 msec >> 95th percentile: 0.000000 msec >> Short term average: 0.000000 msec >> Long term average: 0.000000 msec >> >> Hopefully this starting point will allow others to discuss or contribute changes to >> incrementally process some aspects of northd. We can also decide if it is worth >> progressing with "Part 2" if we think there are potential benefits until we start >> seeing the benefits of DDlog. >> >> Thanks >> >> --- >> v2: Rebase >> v3: Rebase. Fixed compile-time error >> v4: Add additional commits which add framework for incremental processing in northd >> >> Mark Gray (7): >> ovn-northd: Rename ovn-northd.c to northd.c >> northd: Split northd.c >> inc-proc-eng: Allow definition of engine_node with global scope >> northd: Introduce incremental processing for northd >> northd: Add n_nat_entries field to 'struct ovn_datapath' >> northd: Add runtime node >> northd: Add functionality to runtime node >> >> Documentation/tutorials/ovn-openstack.rst | 154 +- >> controller/ovn-controller.c | 2 +- >> lib/inc-proc-eng.h | 24 +- >> northd/automake.mk | 8 + >> northd/en-northd.c | 52 + >> northd/en-northd.h | 17 + >> northd/en-runtime.c | 66 + >> northd/en-runtime.h | 25 + >> northd/inc-proc-northd.c | 258 + >> northd/inc-proc-northd.h | 15 + >> northd/lrouter.dl | 2 +- >> northd/northd.c | 14568 +++++++++++++++++++ >> northd/northd.h | 45 + >> northd/ovn-northd.c | 15084 +------------------- >> northd/ovn.rs | 2 +- >> northd/ovn_northd.dl | 2 +- >> tests/ovn-northd.at | 2 +- >> 17 files changed, 15454 insertions(+), 14872 deletions(-) >> create mode 100644 northd/en-northd.c >> create mode 100644 northd/en-northd.h >> create mode 100644 northd/en-runtime.c >> create mode 100644 northd/en-runtime.h >> create mode 100644 northd/inc-proc-northd.c >> create mode 100644 northd/inc-proc-northd.h >> create mode 100644 northd/northd.c >> create mode 100644 northd/northd.h >> >
Part 1: The first two comits deal with reorganising the northd code base a little. Please note that this commit mainly involves moving code around with minimal code changes. However, due to tight coupling between ovn-northd.c and northd.c, some minor changes were needed. For reference, and to help reviews, please examine the following at a minimum: * Configuration of the probe interval in northd.c (ovsdb_idl_set_probe_interval()) * Passing of "use_parallel_build" and "lflow_locks" from ovn-northd.c and northd.c. * Update of "struct northd_context": additon of fields and move to h file. The commits were (hopefully) structured in a way to make the review easier. As this change touches all of ovn-northd, any change to "master" will make a rebase necessary and probably difficult. Therefore, if the general ideas is OK, then it would be great if this series could be expedited to prevent many rebases! Part 2: The remaining commits add the inc-proc-eng framework to northd. This does *not* add any incremental processing of northd processing at this stage but provides the framework. Even in this base configuration, we see an advantage as northd no longer processes the databases if it has been woken up only to handle, for example, a unixctl command. This can be seen below: $ ovn-appctl -t ovn-northd stopwatch/reset $ for i in {1..10}; do ovn-appctl -t ovn-northd stopwatch/show >/dev/null; done $ ovn-appctl -t ovn-northd stopwatch/show ovnnb_db_run Statistics for 'ovnnb_db_run' Total samples: 0 Maximum: 0 msec Minimum: 0 msec 95th percentile: 0.000000 msec Short term average: 0.000000 msec Long term average: 0.000000 msec Hopefully this starting point will allow others to discuss or contribute changes to incrementally process some aspects of northd. We can also decide if it is worth progressing with "Part 2" if we think there are potential benefits until we start seeing the benefits of DDlog. Thanks --- v2: Rebase v3: Rebase. Fixed compile-time error v4: Add additional commits which add framework for incremental processing in northd Mark Gray (7): ovn-northd: Rename ovn-northd.c to northd.c northd: Split northd.c inc-proc-eng: Allow definition of engine_node with global scope northd: Introduce incremental processing for northd northd: Add n_nat_entries field to 'struct ovn_datapath' northd: Add runtime node northd: Add functionality to runtime node Documentation/tutorials/ovn-openstack.rst | 154 +- controller/ovn-controller.c | 2 +- lib/inc-proc-eng.h | 24 +- northd/automake.mk | 8 + northd/en-northd.c | 52 + northd/en-northd.h | 17 + northd/en-runtime.c | 66 + northd/en-runtime.h | 25 + northd/inc-proc-northd.c | 258 + northd/inc-proc-northd.h | 15 + northd/lrouter.dl | 2 +- northd/northd.c | 14568 +++++++++++++++++++ northd/northd.h | 45 + northd/ovn-northd.c | 15084 +------------------- northd/ovn.rs | 2 +- northd/ovn_northd.dl | 2 +- tests/ovn-northd.at | 2 +- 17 files changed, 15454 insertions(+), 14872 deletions(-) create mode 100644 northd/en-northd.c create mode 100644 northd/en-northd.h create mode 100644 northd/en-runtime.c create mode 100644 northd/en-runtime.h create mode 100644 northd/inc-proc-northd.c create mode 100644 northd/inc-proc-northd.h create mode 100644 northd/northd.c create mode 100644 northd/northd.h