[ovs-dev] [PATCH] jsonrpc: Turn sorting of json objects off in messages

Anton Ivanov anton.ivanov at cambridgegreys.com
Thu Aug 26 05:31:30 UTC 2021


On 25/08/2021 22:12, Ilya Maximets wrote:
> On 8/25/21 5:17 PM, anton.ivanov at cambridgegreys.com wrote:
>> From: Anton Ivanov <anton.ivanov at cambridgegreys.com>
>>
>> A JSON object sort order is by definition arbitrary. OVS
>> parser(s) do not care about object order - the result is
>> loaded into a SHASH losing any order on the wire.
>>
>> Having the objects sorted is a performance penalty, especially
>> for large objects like f.e. lflow state. That is represented
>> as {"table_name":{"uuid":data, "uuid":data, "uuid":data}}
>>
>> Sorting in a case like this has no meaning neither to human,
>> nor to computer.
> There is a meaning for both human and computer in some cases.
>
> While sorting by UUIDs doesn't make a lot of sense, I agree,
> having sorted columns inside the database row is important
> for debugging purposes.  I'm using something like this:
>    sed 's/{"_uuid"/\n{"_uuid"/g'
> very frequently, while inspecting database transactions in
> order to split different rows to different lines, so the
> text editor can work with the result or some other scripts
> can process them in a pipeline.  And this command relies on
> a fact that '_uuid' goes first in a row.
>
> Also, the thing that unit tests are not failing with this
> change is a pure luck, because, IIUC, we do have a fair amount
> of tests that looks for exact order in transactions.  The
> problem here that order will depend on the CPU architecture,
> because different ways of hash computation will be in use.

JSON which has a "special" requirement to be sorted is NOT JSON.

1. The change do not change transaction order, because order of 
operations in transaction is expressed via ARRAYS. Any part which is 
expressed as an object is an arbitrary order.

2. The change is specific solely to JSON RPC which for some reason 
someone at some point decided to have sorted. All other parts of OVS do 
not use sort.

3. There is no benefit to it. Only a penalty which is paid for every 
transaction. Every time.

>
> In general, I'd consider this change as a step back in
> debugability.  So, unless it provides a huge performance
> benefit, I'd like to not have it.  And tests should be
> modified in a way that doesn't require exact order of columns,
> for sure.
>
> I'll give it a shot in a scale test run once I have a chance.
>
> Best regards, Ilya Maximets.
>
> If, however, there are 0.5M such records, it
>> is a subtantial CPU (and latency) penalty.
>>
>> Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>
>> ---
>>   lib/jsonrpc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
>> index c8ce5362e..3b44f73fe 100644
>> --- a/lib/jsonrpc.c
>> +++ b/lib/jsonrpc.c
>> @@ -802,7 +802,7 @@ jsonrpc_msg_to_string(const struct jsonrpc_msg *m)
>>   {
>>       struct jsonrpc_msg *copy = jsonrpc_msg_clone(m);
>>       struct json *json = jsonrpc_msg_to_json(copy);
>> -    char *s = json_to_string(json, JSSF_SORT);
>> +    char *s = json_to_string(json, 0);
>>       json_destroy(json);
>>       return s;
>>   }
>>
>

-- 
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/



More information about the dev mailing list