On 6/20/22 19:04, Ilya Maximets wrote:
> On 11/30/21 10:47, Dumitru Ceara wrote:
>> Clients might be connected to multiple databases (e.g., ovn-controller
>> is connected to OVN_Southbound and Open_vSwitch databases) and the IDL
>> memory statistics are more useful if they're not aggregated.
>>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>>  lib/ovsdb-idl.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
>> index a6323d2b8b9a..26b5aae36a92 100644
>> --- a/lib/ovsdb-idl.c
>> +++ b/lib/ovsdb-idl.c
>> @@ -498,9 +498,20 @@ ovsdb_idl_get_memory_usage(struct ovsdb_idl *idl, 
>> struct simap *usage)
>>          cells += n_rows * n_columns;
>>      }
>>  
>> -    simap_increase(usage, "idl-cells", cells);
>> -    simap_increase(usage, "idl-outstanding-txns",
>> -                   hmap_count(&idl->outstanding_txns));
>> +    struct {
>> +        const char *name;
>> +        unsigned int val;
>> +    } idl_mem_stats[] = {
>> +        {"idl-outstanding-txns", hmap_count(&idl->outstanding_txns)},
>> +        {"idl-cells", cells},
>> +    };
>> +
>> +    for (size_t i = 0; i < ARRAY_SIZE(idl_mem_stats); i++) {
>> +        char *stat_name = xasprintf("%s-%s", idl->class_->database,
>> +                                             idl_mem_stats[i].name);
> 
> Can we swap the database name and the counter name?
> I think it looks better this way.
> 

Sure, that looks better indeed.

> We may also think about just parenthesizing the database
> name instead of using a dash.  It might look better, because
> we will not mix dashes and underscores, e.g.:
> 
> idl-cells(Open_vSwitch):17
> vs
> idl-cells-Open_vSwitch:17
> 
> But I'm really not sure about this one.  What do you think?
> 

My personal opinion is that this is not better than underscores +
dashes.  That makes parsing a tiny bit harder because we potentially
have to escape parenthesis.

So I followed the first suggestion and posted a v2. :)

>> +        simap_increase(usage, stat_name, idl_mem_stats[i].val);
>> +        free(stat_name);
>> +    }
>>  }
>>  
>>  /* Returns a "sequence number" that represents the state of 'idl'.  When
> 

Thanks!

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

Reply via email to