[ovs-dev] [PATCH v2 3/3] ovsdb: monitor: Store serialized json in a json cache.

[Dumitru Ceara]

On 8/30/21 1:23 PM, Ilya Maximets wrote:
> On 8/30/21 12:54 PM, Dumitru Ceara wrote:
>> On 8/24/21 9:00 PM, Ilya Maximets wrote:
>>> 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
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>>> ---
>>>  ovsdb/monitor.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
>>> index 532dedcb6..ab814cf20 100644
>>> --- a/ovsdb/monitor.c
>>> +++ b/ovsdb/monitor.c
>>> @@ -1231,6 +1231,15 @@ ovsdb_monitor_get_update(
>>>                                              condition,
>>>                                              ovsdb_monitor_compose_row_update2);
>>>                  if (!condition || !condition->conditional) {
>>> +                    if (json) {
>>> +                        struct json *json_serialized;
>>> +
>>> +                        /* Pre-serializing the object to avoid doing this
>>> +                         * for every client. */
>>> +                        json_serialized = json_serialized_object_create(json);
>>> +                        json_destroy(json);
>>> +                        json = json_serialized;
>>> +                    }
>>
>> I think this could be part of ovsdb_monitor_json_cache_insert()
>> directly.  It would save a level of indentation here and it seems to me
>> like it's just an implementation decision about how to store the cached
>> json.
> 
> Not really.  ovsdb_monitor_get_update() returns the json object.
> If serialization will be performed inside the ovsdb_monitor_json_cache_insert(),
> we will return non-serialized version here and it will be serialized again
> while sending to the first client.  Swapping of the 'json' object with the
> serialized one here saves the processing time, i.e. saves one extra serialization.

Ah, you're right, please ignore my original comment.

> 
>>
>> Either way:
>>
>> Acked-by: Dumitru Ceara <dceara at redhat.com>
> 
> Thanks!
> 
>>
>>>                      ovsdb_monitor_json_cache_insert(dbmon, version, mcs,
>>>                                                      json);
>>>                  }
>>>
>>
>