Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.
The FDP team is no longer accepting new bugs in Bugzilla. Please report your issues under FDP project in Jira. Thanks.

Bug 1990069

Summary: [RFE] ovsdb-server: Optimize json string serialization to speed up compaction
Product: Red Hat Enterprise Linux Fast Datapath Reporter: Ilya Maximets <i.maximets>
Component: ovsdb2.16Assignee: Ilya Maximets <i.maximets>
Status: CLOSED ERRATA QA Contact: qding
Severity: high Docs Contact:
Priority: high    
Version: RHEL 8.0CC: ctrautma, jhsiao, jishi, kfida, ralongi
Target Milestone: ---Keywords: FutureFeature
Target Release: FDP 21.I   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: openvswitch2.16-2.16.0-6.el8fdp Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2022-01-10 16:50:58 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Ilya Maximets 2021-08-04 16:48:39 UTC
OVS databases contains lots of strings, so the json_serialize_string()
function is a fairly hot spot in the database compaction process.

One of the things that this function does is that it checks if the
character needs to be replaced with the corresponding escape sequence
in order to match JSON RFC and to avoid obvious problems.  The problem
is that implementation scans and checks symbols one by one and copies
them to the destination dynamic string one by one.  One of the key
optimization points can be to collect sequences of characters that
doesn't need to be replaced and copy them all at once with a better
memory copy methods.  Early tests shows that this method speeds up
the database compaction in total by 8-9%.

Comment 1 OvS team 2021-09-01 22:02:43 UTC
* Tue Aug 31 2021 Ilya Maximets <i.maximets> - 2.16.0-6
- ovsdb: monitor: Store serialized json in a json cache. [RH git: bc20330c85] (#1996152)
    commit 43e66fc27659af2a5c976bdd27fe747b442b5554
    Author: Ilya Maximets <i.maximets>
    Date:   Tue Aug 24 21:00:39 2021 +0200
    
        Same json from a json cache is typically sent to all the clients,
        e.g., in case of OVN deployment with ovn-monitor-all=true.
    
        There could be hundreds or thousands connected clients and ovsdb
        will serialize the same json object for each of them before sending.
    
        Serializing it once before storing into json cache to speed up
        processing.
    
        This change allows to save a lot of CPU cycles and a bit of memory
        since we need to store in memory only a string and not the full json
        object.
    
        Testing with ovn-heater on 120 nodes using density-heavy scenario
        shows reduction of the total CPU time used by Southbound DB processes
        from 256 minutes to 147.  Duration of unreasonably long poll intervals
        also reduced dramatically from 7 to 2 seconds:
    
                   Count   Min    Max   Median    Mean   95 percentile
         -------------------------------------------------------------
          Before   1934   1012   7480   4302.5   4875.3     7034.3
          After    1909   1004   2730   1453.0   1532.5     2053.6
    
        Acked-by: Dumitru Ceara <dceara>
        Acked-by: Han Zhou <hzhou>
        Signed-off-by: Ilya Maximets <i.maximets>
    
    Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1996152
    Signed-off-by: Ilya Maximets <i.maximets>


* Tue Aug 31 2021 Ilya Maximets <i.maximets> - 2.16.0-5
- raft: Don't keep full json objects in memory if no longer needed. [RH git: 4606423e8b] (#1990058)
    commit 0de882954032aa37dc943bafd72c33324aa0c95a
    Author: Ilya Maximets <i.maximets>
    Date:   Tue Aug 24 21:00:38 2021 +0200
    
        raft: Don't keep full json objects in memory if no longer needed.
    
        Raft log entries (and raft database snapshot) contains json objects
        of the data.  Follower receives append requests with data that gets
        parsed and added to the raft log.  Leader receives execution requests,
        parses data out of them and adds to the log.  In both cases, later
        ovsdb-server reads the log with ovsdb_storage_read(), constructs
        transaction and updates the database.  On followers these json objects
        in common case are never used again.  Leader may use them to send
        append requests or snapshot installation requests to followers.
        However, all these operations (except for ovsdb_storage_read()) are
        just serializing the json in order to send it over the network.
    
        Json objects are significantly larger than their serialized string
        representation.  For example, the snapshot of the database from one of
        the ovn-heater scale tests takes 270 MB as a string, but 1.6 GB as
        a json object from the total 3.8 GB consumed by ovsdb-server process.
    
        ovsdb_storage_read() for a given raft entry happens only once in a
        lifetime, so after this call, we can serialize the json object, store
        the string representation and free the actual json object that ovsdb
        will never need again.  This can save a lot of memory and can also
        save serialization time, because each raft entry for append requests
        and snapshot installation requests serialized only once instead of
        doing that every time such request needs to be sent.
    
        JSON_SERIALIZED_OBJECT can be used in order to seamlessly integrate
        pre-serialized data into raft_header and similar json objects.
    
        One major special case is creation of a database snapshot.
        Snapshot installation request received over the network will be parsed
        and read by ovsdb-server just like any other raft log entry.  However,
        snapshots created locally with raft_store_snapshot() will never be
        read back, because they reflect the current state of the database,
        hence already applied.  For this case we can free the json object
        right after writing snapshot on disk.
    
        Tests performed with ovn-heater on 60 node density-light scenario,
        where on-disk database goes up to 97 MB, shows average memory
        consumption of ovsdb-server Southbound DB processes decreased by 58%
        (from 602 MB to 256 MB per process) and peak memory consumption
        decreased by 40% (from 1288 MB to 771 MB).
    
        Test with 120 nodes on density-heavy scenario with 270 MB on-disk
        database shows 1.5 GB memory consumption decrease as expected.
        Also, total CPU time consumed by the Southbound DB process reduced
        from 296 to 256 minutes.  Number of unreasonably long poll intervals
        reduced from 2896 down to 1934.
    
        Deserialization is also implemented just in case.  I didn't see this
        function being invoked in practice.
    
        Acked-by: Dumitru Ceara <dceara>
        Acked-by: Han Zhou <hzhou>
        Signed-off-by: Ilya Maximets <i.maximets>
    
    Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1990058
    Signed-off-by: Ilya Maximets <i.maximets>


* Tue Aug 31 2021 Ilya Maximets <i.maximets> - 2.16.0-4
- json: Add support for partially serialized json objects. [RH git: 885e5ce1b5] (#1990058)
    commit b0bca6f27aae845c3ca8b48d66a7dbd3d978162a
    Author: Ilya Maximets <i.maximets>
    Date:   Tue Aug 24 21:00:37 2021 +0200
    
        json: Add support for partially serialized json objects.
    
        Introducing a new json type JSON_SERIALIZED_OBJECT.  It's not an
        actual type that can be seen in a json message on a wire, but
        internal type that is intended to hold a serialized version of
        some other json object.  For this reason it's defined after the
        JSON_N_TYPES to not confuse parsers and other parts of the code
        that relies on compliance with RFC 4627.
    
        With this JSON type internal users may construct large JSON objects,
        parts of which are already serialized.  This way, while serializing
        the larger object, data from JSON_SERIALIZED_OBJECT can be added
        directly to the result, without additional processing.
    
        This will be used by next commits to add pre-serialized JSON data
        to the raft_header structure, that can be converted to a JSON
        before writing the file transaction on disk or sending to other
        servers.  Same technique can also be used to pre-serialize json_cache
        for ovsdb monitors, this should allow to not perform serialization
        for every client and will save some more memory.
    
        Since serialized JSON is just a string, reusing the 'json->string'
        pointer for it.
    
        Acked-by: Dumitru Ceara <dceara>
        Acked-by: Han Zhou <hzhou>
        Signed-off-by: Ilya Maximets <i.maximets>
    
    Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1990058
    Signed-off-by: Ilya Maximets <i.maximets>


* Tue Aug 31 2021 Ilya Maximets <i.maximets> - 2.16.0-3
- json: Optimize string serialization. [RH git: bb1654da63] (#1990069)
    commit 748010ff304b7cd2c43f4eb98a554433f0df07f9
    Author: Ilya Maximets <i.maximets>
    Date:   Tue Aug 24 23:07:22 2021 +0200
    
        json: Optimize string serialization.
    
        Current string serialization code puts all characters one by one.
        This is slow because dynamic string needs to perform length checks
        on every ds_put_char() and it's also doesn't allow compiler to use
        better memory copy operations, i.e. doesn't allow copying few bytes
        at once.
    
        Special symbols are rare in a typical database.  Quotes are frequent,
        but not too frequent.  In databases created by ovn-kubernetes, for
        example, usually there are at least 10 to 50 chars between quotes.
        So, it's better to count characters that doesn't require escaping
        and use fast data copy for the whole sequential block.
    
        Testing with a synthetic benchmark (included) on my laptop shows
        following performance improvement:
    
           Size      Q  S       Before       After       Diff
         -----------------------------------------------------
         100000      0  0 :    0.227 ms     0.142 ms   -37.4 %
         100000      2  1 :    0.277 ms     0.186 ms   -32.8 %
         100000      10 1 :    0.361 ms     0.309 ms   -14.4 %
         10000000    0  0 :   22.720 ms    12.160 ms   -46.4 %
         10000000    2  1 :   27.470 ms    19.300 ms   -29.7 %
         10000000    10 1 :   37.950 ms    31.250 ms   -17.6 %
         100000000   0  0 :  239.600 ms   126.700 ms   -47.1 %
         100000000   2  1 :  292.400 ms   188.600 ms   -35.4 %
         100000000   10 1 :  387.700 ms   321.200 ms   -17.1 %
    
        Here Q - probability (%) for a character to be a '\"' and
        S - probability (%) to be a special character ( < 32).
    
        Testing with a closer to real world scenario shows overall decrease
        of the time needed for database compaction by ~5-10 %.  And this
        change also decreases CPU consumption in general, because string
        serialization is used in many different places including ovsdb
        monitors and raft.
    
        Signed-off-by: Ilya Maximets <i.maximets>
        Acked-by: Numan Siddique <numans>
        Acked-by: Dumitru Ceara <dceara>
    
    Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1990069
    Signed-off-by: Ilya Maximets <i.maximets>

Comment 4 Jianlin Shi 2022-01-05 07:13:13 UTC
Hi llya,

Could you give some suggestions about how to test the feature? thanks

Comment 5 Ilya Maximets 2022-01-05 13:48:11 UTC
(In reply to Jianlin Shi from comment #4)
> Could you give some suggestions about how to test the feature? thanks

Hi.  This BZ is about 'json: Optimize string serialization.' patch.
Unfortunately, it's a pure performance optimization and its performance
impact is not easy to isolate without a synthetic unit test.

One option would be to get a fairly big OVN Southbound database file
(I can give you one if needed) and execute the following command:

  ovsdb-tool compact <database file>

It's still hard to see the difference, because the actual json serialization
in this test would take only about 10% of the execution time, and optimization
saves about 10% of that time.  So, we're looking for ~1% of the overall
performance difference for the 'ovsdb-tool compact <database file>' call.
The performance impact is more visible in a high load tests with big number
of actual ovsdb clients, but that is likely not what you're looking for.

On a 495 MB database file 'ovsdb-tool compact' works ~47 seconds on my system,
with the patch applied it goes down to ~46.5.  Actual serialization in this
test takes ~4 and ~3.5 seconds respectively (I had to modify some code to
measure that).
You may also run the above commend under 'perf record -- <command>' and look
at the percentage for the 'json_serialize_string' function in the output of
'perf report'.  In my testing it goes down from ~3.8% to ~2.8%.

Another option is to actually start an ovsdb-server with that big database
file and request a compaction with 'ovs-appctl ovsdb-server/compact' and
check how log compaction takes.  This will remove the reading of the database
from file from the measures, so the performance impact should be more visible.

Comment 6 Jianlin Shi 2022-01-06 07:57:04 UTC
try to configure ovn with following script:


ovn-nbctl ls-add public
ovn-nbctl lsp-add public ln_p1                                     
ovn-nbctl lsp-set-addresses ln_p1 unknown
ovn-nbctl lsp-set-type ln_p1 localnet
ovn-nbctl lsp-set-options ln_p1 network_name=nattest
      
i=1   
for m in `seq 0 9`;do   
  for n in `seq 1 99`;do
    ovn-nbctl lr-add r${i}
    ovn-nbctl lrp-add r${i} r${i}_public 00:de:ad:ff:$m:$n 172.16.$m.$n/16
    ovn-nbctl lrp-add r${i} r${i}_s${i} 00:de:ad:fe:$m:$n 173.$m.$n.1/24
    ovn-nbctl lr-nat-add r${i} dnat_and_snat 172.16.${m}.$((n+100)) 173.$m.$n.2
    ovn-nbctl lrp-set-gateway-chassis r${i}_public hv1          
                         
                # s1                                       
    ovn-nbctl ls-add s${i}                        
                                    
                # s1 - r1                            
    ovn-nbctl lsp-add s${i} s${i}_r${i}
    ovn-nbctl lsp-set-type s${i}_r${i} router
    ovn-nbctl lsp-set-addresses s${i}_r${i} router              
    ovn-nbctl lsp-set-options s${i}_r${i} router-port=r${i}_s${i}
                # s1 - vm1                                 
    ovn-nbctl lsp-add s$i vm$i                    
    ovn-nbctl lsp-set-addresses vm$i "00:de:ad:01:$m:$n 173.$m.$n.2"
    ovn-nbctl lrp-add r$i r${i}_public 40:44:00:00:$m:$n 172.16.$m.$n/16
    ovn-nbctl lsp-add public public_r${i}
    ovn-nbctl lsp-set-type public_r${i} router
    ovn-nbctl lsp-set-addresses public_r${i} router
                       
    ovn-nbctl lsp-set-options public_r${i} router-port=r${i}_public           
    let i++
    if [ $i -gt 600 ];then
       break;                                                     
    fi                   
  done                                                     
  if [ $i -gt 600 ];then                             
    break;                          
  fi                                                 
done                                                
#add host vm1                                       
ip netns add vm1
ovs-vsctl add-port br-int vm1 -- set interface vm1 type=internal
ip link set vm1 netns vm1 
ip netns exec vm1 ip link set vm1 address 00:de:ad:01:00:01
ip netns exec vm1 ip addr add 173.0.1.2/24 dev vm1
ip netns exec vm1 ip link set vm1 up
ovs-vsctl set Interface vm1 external_ids:iface-id=vm1

ip netns add vm2
ovs-vsctl add-port br-int vm2 -- set interface vm2 type=internal
ip link set vm2 netns vm2 
ip netns exec vm2 ip link set vm2 address 00:de:ad:01:00:02
ip netns exec vm2 ip addr add 173.0.2.2/24 dev vm2
ip netns exec vm2 ip link set vm2 up
ovs-vsctl set Interface vm2 external_ids:iface-id=vm2

#set provide network
ovs-vsctl add-br nat_test
ip link set nat_test up
ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=nattest:nat_test

ip netns add vm0
ovs-vsctl add-port nat_test vm0 -- set interface vm0 type=internal
ip link set vm0 netns vm0
ip netns exec vm0 ip link set vm0 address 00:00:00:00:00:01
ip netns exec vm0 ip addr add 172.16.0.100/16 dev vm0
ip netns exec vm0 ip link set vm0 up
ovs-vsctl set Interface vm0 external_ids:iface-id=vm0
ip netns exec vm1 ip route add default via 173.0.1.1
ip netns exec vm2 ip route add default via 173.0.2.1

then compact the sbdb: time ovs-appctl -t /var/run/ovn/ovnsb_db.ctl ovsdb-server/compact

on openvswitch2.16-1:

[root@wsfd-advnetlab16 bz1990069]# time ovs-appctl -t /var/run/ovn/ovnsb_db.ctl ovsdb-server/compact  

real    0m12.029s                                                                                     
user    0m0.005s                                                                                      
sys     0m0.007s

[root@wsfd-advnetlab16 bz1990069]# du -sh /var/lib/ovn/ovnsb_db.db                                    
512M    /var/lib/ovn/ovnsb_db.db

on openvswitch2.16-6:

[root@wsfd-advnetlab16 bz1990069]# time ovs-appctl -t /var/run/ovn/ovnsb_db.ctl ovsdb-server/compact
                                                                                                      
real    0m11.445s                                                                                     
user    0m0.006s                                                                                      
sys     0m0.006s

set VERIFIED

Comment 8 errata-xmlrpc 2022-01-10 16:50:58 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (openvswitch2.16 bug fix update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:0053