northd: Incrementally process SB.Load_balancer updates.

Commit 9deb000 ("northd: Remove potential duplicates in SB
Load_Balancer table.") added code to northd to ensure that SB duplicates
get removed.  Quoting from that commit message:

    The Southbound Load_Balancer table (like the Northbound one) doesn't
    define an index (e.g., by name).  This essentially means that there can
    be duplicate records in the database.  Moreover, the OVSDB RAFT
    implementation ensures "at-least-once" consistency [0] making it
    possible for such duplicates to "spontaneously" appear.

That is still the case, however, since then ovn-northd started to
incrementally process (NB) load balancer updates and self-created
SB.Load_Balancer records trigger a recompute of the "en_sync_to_sb_lb"
node.  At scale (e.g., an OpenShift cluster with 750 nodes and 64K load
balancers) this is significant:
  inc_proc_eng|INFO|node: sync_to_sb_lb, recompute (missing handler for input SB_load_balancer) took 524ms

All we actually care about is whether the transaction to create the SB
record created a spurious clone.  Instead of triggering a recompute, add
a new handler for this type of SB update and check for duplicates.  With
the database from the same scale test as above the duplicate check is
almost instantaneous.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Ales Musil <amusil@redhat.com>
Acked-by: Numan Siddique <numans@ovn.org>

northd/en-sync-sb.c

@@ -249,6 +249,23 @@ sync_to_sb_lb_northd_handler(struct engine_node *node, void *data OVS_UNUSED)
     return true;
 }
 
+bool
+sync_to_sb_lb_sb_load_balancer(struct engine_node *node, void *data OVS_UNUSED)
+{
+    const struct sbrec_load_balancer_table *sb_load_balancer_table =
+        EN_OVSDB_GET(engine_get_input("SB_load_balancer", node));
+
+    /* The only reason to handle SB.Load_Balancer updates is to detect
+     * spurious records being created in clustered databases due to
+     * lack of indexing on the SB.Load_Balancer table.  All other changes
+     * are valid and performed by northd, the only write-client for
+     * this table. */
+    if (check_sb_lb_duplicates(sb_load_balancer_table)) {
+        return false;
+    }
+    return true;
+}
+
 /* sync_to_sb_pb engine node functions.
  * This engine node syncs the SB Port Bindings (partly).
  * en_northd engine create the SB Port binding rows and

northd/en-sync-sb.h

@@ -21,6 +21,8 @@ void *en_sync_to_sb_lb_init(struct engine_node *, struct engine_arg *);
 void en_sync_to_sb_lb_run(struct engine_node *, void *data);
 void en_sync_to_sb_lb_cleanup(void *data);
 bool sync_to_sb_lb_northd_handler(struct engine_node *, void *data OVS_UNUSED);
+bool sync_to_sb_lb_sb_load_balancer(struct engine_node *,
+                                    void *data OVS_UNUSED);
 
 void *en_sync_to_sb_pb_init(struct engine_node *, struct engine_arg *);
 void en_sync_to_sb_pb_run(struct engine_node *, void *data);

northd/inc-proc-northd.c

@@ -230,7 +230,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
 
     engine_add_input(&en_sync_to_sb_lb, &en_northd,
                      sync_to_sb_lb_northd_handler);
-    engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer, NULL);
+    engine_add_input(&en_sync_to_sb_lb, &en_sb_load_balancer,
+                     sync_to_sb_lb_sb_load_balancer);
 
     engine_add_input(&en_sync_to_sb_pb, &en_northd,
                      sync_to_sb_pb_northd_handler);

northd/northd.c

@@ -4731,6 +4731,26 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
     }
 }
 
+bool
+check_sb_lb_duplicates(const struct sbrec_load_balancer_table *table)
+{
+    struct sset existing_nb_lb_uuids =
+        SSET_INITIALIZER(&existing_nb_lb_uuids);
+    const struct sbrec_load_balancer *sbrec_lb;
+    bool duplicates = false;
+
+    SBREC_LOAD_BALANCER_TABLE_FOR_EACH (sbrec_lb, table) {
+        const char *nb_lb_uuid = smap_get(&sbrec_lb->external_ids, "lb_id");
+        if (nb_lb_uuid && !sset_add(&existing_nb_lb_uuids, nb_lb_uuid)) {
+            duplicates = true;
+            break;
+        }
+    }
+
+    sset_destroy(&existing_nb_lb_uuids);
+    return duplicates;
+}
+
 /* Syncs the SB port binding for the ovn_port 'op'.  Caller should make sure
  * that the OVN SB IDL txn is not NULL.  Presently it only syncs the nat
  * column of port binding corresponding to the 'op->nbsp' */

northd/northd.h

@@ -369,6 +369,7 @@ void sync_lbs(struct ovsdb_idl_txn *, const struct sbrec_load_balancer_table *,
               struct ovn_datapaths *lr_datapaths,
               struct hmap *lbs,
               struct chassis_features *chassis_features);
+bool check_sb_lb_duplicates(const struct sbrec_load_balancer_table *);
 
 void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports);
 bool sync_pbs_for_northd_ls_changes(struct tracked_ls_changes *);