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.

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 <[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;
  }



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

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to