There are options for that. Debug mode should never be a default.
On 30 August 2021 12:29:16 BST, Ilya Maximets <[email protected]> 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, [email protected] wrote: >>>> From: Anton Ivanov <[email protected]> >>>> >>>> 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 <[email protected]> >>>> --- >>>> 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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
