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

Anton Ivanov anton.ivanov at kot-begemot.co.uk
Mon Aug 30 14:59:07 UTC 2021


There are options for that.

Debug mode should never be a default. 

On 30 August 2021 12:29:16 BST, Ilya Maximets <i.maximets at ovn.org> wrote:
>On 8/26/21 7:31 AM, Anton Ivanov wrote:
>> 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.
>
>There is no requirement, but there is convenience in having them sorted.
>
>> 
>> 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.
>
>I didn't say it changes transaction order, I said that it changes
>order of columns in rows.
>
>> 
>> 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.
>
>I believe it was done for debugability reasons.  The same that I
>have described.
>
>> 
>> 3. There is no benefit to it. Only a penalty which is paid for every transaction. Every time.
>
>As I described previously, there is a benefit.  It's not a performance,
>obviously, but convenience and higher debugability.
>
>> 
>>>
>>> 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;
>>>>   }
>>>>
>>>
>> 
>
>

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


More information about the dev mailing list