[ovs-dev] [PATCH v3 00/17] Daemon mode for ovn-nbctl

Mon Jul 16 18:11:19 UTC 2018

I've had a look through again and this series addresses the findings I 
previously had, so in short:

Acked-by: Mark Michelson <mmichels at redhat.com>

I decided to do some testing to see what sort of performance gain we see 
with this patch. I'm attaching four scripts that I used for testing.

In all of the tests, I create a logical router and 159 logical switches 
connected to the router. I then create 92 logical switch ports on each 
logical switch (a total of 14628 ports). On alternating logical switch 
port additions, I create an address set. Every set of two logical switch 
ports gets added to the most recently created address set. For each 
logical switch port, I also create two ACLs. Note that the script does 
not perform any synchronization or port binding. The goal of the script 
is to isolate the performance of ovn-nbctl at scale rather than as an 
overall view of the performance of OVN.

The scale.sh script is a straightforward version that uses individual 
calls to ovn-nbctl at each step. The scale_batch.sh script batches 
multiple operations into a single ovn-nbctl call. This way, the switch 
port, address set, and ACLs are created in one ovn-nbctl invocation. I 
ran these scripts about a month ago in a sandbox against OVS master. The 
scale.sh script took ~13 hours to complete. The scale_batch.sh script 
took ~3 hours to complete.

I then modified each of these scripts to work with ovn-nbctl in daemon 
mode. scale.sh was modified into scale-daemon.sh, and scale_batch.sh was 
modified into scale_batch-daemon.sh. I applied the patch series to the 
current OVS master and ran the daemon versions of the scripts in a 
sandbox. scale-daemon.sh completed in 8 minutes 54 seconds. 
scale_batch-daemon.sh completed in 3 minutes 52 seconds. In both cases, 
this is a 99.9% decrease in the amount of time to complete.

On 07/13/2018 02:36 PM, Jakub Sitnicki wrote:
> This series extends ovn-nbctl tool with support for the daemon mode, where
> ovn-nbctl acts a long-lived process that accepts commands over a UNIX socket.
> The daemon can be started the same way as any other OVS/OVN server:
> 
>    ovn-nbctl --detach --pidfile --log-file
> 
> While commands can be issued to it using the 'ovs-appctl' tool:
> 
>    ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
> 
> Although, the plan for future work is to extend ovn-nbctl so that it can control
> the daemon, if one is running.
> 
> The motivation and the main benefit from the daemon mode is that the contents of
> NB database have to be obtained only once, when the first command is ran. On big
> databases (1000's of logical ports) this results in a speed up per command in
> the range of 100's of milliseconds.
> 
> The changes are functional to the point that all test cases in the ovn-nbctl
> test suite (tests/ovn-nbctl.at) and OVN test suite (tests/ovn.at) pass when
> running ovn-nbctl as a daemon [1].
> 
> However, I'm still thinking how to nicely integrate the daemon mode with the
> test suite so that the existing tests can be run using either the normal or the
> daemon mode. Any ideas?
> 
> A dirty, just-for-development integration with tests is available at:
> 
>    https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-13
> 
> Also, not all places that call ctl_fatal() have been converted yet to propagate
> the error to the caller. As a result hitting an unconverted an error path will
> cause the daemon process to die. This can be worked around with the --monitor
> option.
> 
> Daemon mode should be considered experimental.
> 
> Changes v2 -> v3:
> 
> - Wait for IDL to connect before detaching and running the unixctl server. Also,
>    terminate the daemon when the connection to the NB DB has failed. This
>    behavior is modeled after ovn-trace daemon mode.
> 
> - Add a dedicated option parser for the daemon. Parse only the options that are
>    have an effect on the main loop. Logging options are not supported. Usual way
>    to change the log levels for daemons is with 'vlog/set' command. Reported by
>    Mark Michelson.
> 
> - Drop the test for 'sync' command. The test is flawed as pointed out by Mark
>    Michelson. Reading the '*_cfg' value after 'ovn-nbctl --wait=X sync' has
>    returned does not prove that '--wait' has blocked until the '*_cfg' value has
>    changed. At the same time, we cannot read out the updated '*_cfg' value in the
>    same transaction as we change it in.
> 
> - To review just the interdiff run:
> 
>    git fetch --tags https://github.com/jsitnicki/ovs
>    git diff wip-nbctl-daemon-2018-07-{12,13}
> 
> Changes v1 -> v2:
> 
> - Work around a limitation in GCC 4.8.5 (RHEL/CentOS 7.5) that doesn't let us
>    use a constant expression as a RHS value in a structure assigment. Found by
>    the 0-day bot.
> 
> Changes RFC -> v1:
> 
> - Rebase onto master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
> 
> - Add support for commands that use tabular output ('find' and 'list') thanks to
>    recent work in table module by Ben Pfaff. This includes support for options
>    that control table formatting.
> 
> - Run prerequisites routines. In the end this is needed to support the 'sync'
>    command, which itself is more like an option than command (has to get
>    processed before the commands run). This is also the reason for minor changes
>    in the IDL.
> 
> - Add support for --dry-run, --wait / --no-wait, --timeout, and --oneline
>    options. Timeout handling is different than in the normal mode - we only time
>    out in poll_block(). Still, it serves the purpose avoiding an infinite 'sync'.
> 
> - Add tests for ovn-nbctl 'sync' command, dry-run mode, and oneline-formatted
>    output.
> 
> - Extend the ovn-nbctl man-page with description of daemon mode.
> 
> - Remove extraneous return at the end of a void function. Pointed out by Ben
>    Pfaff.
> 
> - Drop WIP patch for integration with tests from the series until a right
>    approach can be found. Integration with tests that was used during development
>    is available at:
> 
>    https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-11
> 
> - Minor style cleanups.
> 
> Thanks,
> Jakub
> 
> [1] Except test "2563: ovn -- IPv6 ND Router Solicitation responder" which seems
>      broken in master @ 61b1c7acb9a2 ("netdev-bsd: Fix crash on FreeBSD.").
> 
> Jakub Sitnicki (17):
>    table: Introduce a constant for default table style.
>    ovsdb-idl: Allow monitoring columns that are already monitored.
>    ovn-nbctl: Extract the main loop.
>    ovn-nbctl: Pull up destroying commands from do_nbctl().
>    ovn-nbctl: Pull up releasing IDL from do_nbctl().
>    ovn-nbctl: Signal need to try again via an output param.
>    ovn-nbctl: Propagate the error from do_nbctl().
>    ovn-nbctl: Propagate errors from the main loop.
>    ovn-nbctl: Propagate errors from prerequisites runner.
>    ovn-nbctl: Introduce a poll_timer based wait timeout.
>    ovn-nbctl: Extract helper for printing oneline output.
>    ovn-nbctl: Extract handling of options that affect main loop.
>    ovn-nbctl: Extract a helper for building short options string.
>    ovn-nbctl: Extract a helper for appending command options.
>    ovn-nbctl: Initial support for daemon mode.
>    tests: Add test for ovn-nbctl dry run mode.
>    tests: Add test for oneline-formatted output for ovn-nbctl.
> 
>   lib/ovsdb-idl.c               |  16 +-
>   lib/table.h                   |   2 +
>   ovn/utilities/ovn-nbctl.8.xml |  40 +++
>   ovn/utilities/ovn-nbctl.c     | 654 +++++++++++++++++++++++++++++++++---------
>   tests/ovn-nbctl.at            |  42 +++
>   5 files changed, 622 insertions(+), 132 deletions(-)
> 
> --
> 2.14.4
> 