On Mon, Dec 14, 2020 at 8:37 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
>
> There could be hundreds or thousands of ct zones in external-ids map.
> Iteration over all of them and reconstruction of the whole new map
> is unnecessary and only hurts performance of both ovn-controller and
> ovsdb-server on the node. Replacing with partial map updates to avoid
> unnecessary work.
>
> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
Thanks Ilya for this improvement patch. I applied to master and also
to branch-20.12
since this patch has performance improvements.
Thanks
Numan
> ---
> controller/ovn-controller.c | 47 +++----------------------------------
> 1 file changed, 3 insertions(+), 44 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index b5f0c1315..7540e2eee 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -729,9 +729,6 @@ static void
> commit_ct_zones(const struct ovsrec_bridge *br_int,
> struct shash *pending_ct_zones)
> {
> - struct smap ct_add_ids = SMAP_INITIALIZER(&ct_add_ids);
> - struct sset ct_del_ids = SSET_INITIALIZER(&ct_del_ids);
> -
> struct shash_node *iter;
> SHASH_FOR_EACH(iter, pending_ct_zones) {
> struct ct_zone_pending_entry *ctzpe = iter->data;
> @@ -750,57 +747,19 @@ commit_ct_zones(const struct ovsrec_bridge *br_int,
> struct smap_node *node =
> smap_get_node(&br_int->external_ids, user_str);
> if (!node || strcmp(node->value, zone_str)) {
> - smap_add_nocopy(&ct_add_ids, user_str, zone_str);
> - user_str = NULL;
> - zone_str = NULL;
> + ovsrec_bridge_update_external_ids_setkey(br_int,
> + user_str, zone_str);
> }
> free(zone_str);
> } else {
> if (smap_get(&br_int->external_ids, user_str)) {
> - sset_add(&ct_del_ids, user_str);
> + ovsrec_bridge_update_external_ids_delkey(br_int, user_str);
> }
> }
> free(user_str);
>
> ctzpe->state = CT_ZONE_DB_SENT;
> }
> -
> - /* Update the bridge external IDs only if really needed (i.e., we must
> - * add a value or delete one). Rebuilding the external IDs map at
> - * every run is a costly operation when having lots of ct_zones.
> - */
> - if (!smap_is_empty(&ct_add_ids) || !sset_is_empty(&ct_del_ids)) {
> - struct smap new_ids = SMAP_INITIALIZER(&new_ids);
> -
> - struct smap_node *id;
> - SMAP_FOR_EACH (id, &br_int->external_ids) {
> - if (sset_find_and_delete(&ct_del_ids, id->key)) {
> - continue;
> - }
> -
> - if (smap_get(&ct_add_ids, id->key)) {
> - continue;
> - }
> -
> - smap_add(&new_ids, id->key, id->value);
> - }
> -
> - struct smap_node *next_id;
> - SMAP_FOR_EACH_SAFE (id, next_id, &ct_add_ids) {
> - smap_replace(&new_ids, id->key, id->value);
> - smap_remove_node(&ct_add_ids, id);
> - }
> -
> - ovsrec_bridge_verify_external_ids(br_int);
> - ovsrec_bridge_set_external_ids(br_int, &new_ids);
> - smap_destroy(&new_ids);
> - }
> -
> - ovs_assert(smap_is_empty(&ct_add_ids));
> - ovs_assert(sset_is_empty(&ct_del_ids));
> -
> - smap_destroy(&ct_add_ids);
> - sset_destroy(&ct_del_ids);
> }
>
> static void
> --
> 2.25.4
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
