Commit 9deb000536e0 ("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 <dce...@redhat.com>
---
northd/en-sync-sb.c | 17 +++++++++++++++++
northd/en-sync-sb.h | 2 ++
northd/inc-proc-northd.c | 3 ++-
northd/northd.c | 21 +++++++++++++++++++++
northd/northd.h | 1 +
5 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/northd/en-sync-sb.c b/northd/en-sync-sb.c
index aae396a43d..8c07a71ee8 100644
--- a/northd/en-sync-sb.c
+++ b/northd/en-sync-sb.c
@@ -248,6 +248,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
diff --git a/northd/en-sync-sb.h b/northd/en-sync-sb.h
index f08565eee1..3bcbb82594 100644
--- a/northd/en-sync-sb.h
+++ b/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);
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 8b08171179..04df0b06c2 100644
--- a/northd/inc-proc-northd.c
+++ b/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);
diff --git a/northd/northd.c b/northd/northd.c
index 528027d07b..ead337219d 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4660,6 +4660,27 @@ 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,
+ CONST_CAST(void *, 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' */
diff --git a/northd/northd.h b/northd/northd.h
index 05f4140d77..b30e005cf1 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -365,6 +365,7 @@ const struct ovn_datapath *northd_get_datapath_for_port(
const struct hmap *ls_ports, const char *port_name);
void sync_lbs(struct ovsdb_idl_txn *, const struct sbrec_load_balancer_table *,
struct ovn_datapaths *ls_datapaths, struct hmap *lbs);
+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 *);
--
2.39.3
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
