From: Martin Schwenke <martin@meltin.net>
Date: Thu, 23 Apr 2020 08:51:40 +0000 (+1000)
Subject: ctdb-daemon: Add separate hot keys array for database statistics
X-Git-Tag: ldb-2.2.0~361
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21b9844bcbb4d8c18d82df063d6c143b435cdfb7;p=thirdparty%2Fsamba.git

ctdb-daemon: Add separate hot keys array for database statistics

There are 2 reasons for this.  Sorting of hot keys is broken and will
be changed to an implementation that needs a named (i.e. not
anonymous) structure.  Also, at least one non-protocol field will be
added to facilitate more useful logging.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
---

diff --git a/ctdb/include/ctdb_private.h b/ctdb/include/ctdb_private.h
index e532323bb34..3eb536672b2 100644
--- a/ctdb/include/ctdb_private.h
+++ b/ctdb/include/ctdb_private.h
@@ -342,6 +342,11 @@ struct ctdb_context {
 	struct lock_context *lock_pending;
 };
 
+struct ctdb_db_hot_key {
+	uint32_t count;
+	TDB_DATA key;
+};
+
 struct ctdb_db_context {
 	struct ctdb_db_context *next, *prev;
 	struct ctdb_context *ctdb;
@@ -375,6 +380,7 @@ struct ctdb_db_context {
 	struct trbt_tree *defer_dmaster;
 
 	struct ctdb_db_statistics_old statistics;
+	struct ctdb_db_hot_key hot_keys[MAX_HOT_KEYS];
 
 	struct lock_context *lock_current;
 	struct lock_context *lock_pending;
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index f8bf2d8d41e..485b0b3bd32 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -828,23 +828,23 @@ ctdb_update_db_stat_hot_keys(struct ctdb_db_context *ctdb_db, TDB_DATA key,
 	char *keystr;
 
 	/* smallest value is always at index 0 */
-	if (count <= ctdb_db->statistics.hot_keys[0].count) {
+	if (count <= ctdb_db->hot_keys[0].count) {
 		return;
 	}
 
 	/* see if we already know this key */
 	for (i = 0; i < MAX_HOT_KEYS; i++) {
-		if (key.dsize != ctdb_db->statistics.hot_keys[i].key.dsize) {
+		if (key.dsize != ctdb_db->hot_keys[i].key.dsize) {
 			continue;
 		}
-		if (memcmp(key.dptr, ctdb_db->statistics.hot_keys[i].key.dptr, key.dsize)) {
+		if (memcmp(key.dptr, ctdb_db->hot_keys[i].key.dptr, key.dsize)) {
 			continue;
 		}
 		/* found an entry for this key */
-		if (count <= ctdb_db->statistics.hot_keys[i].count) {
+		if (count <= ctdb_db->hot_keys[i].count) {
 			return;
 		}
-		ctdb_db->statistics.hot_keys[i].count = count;
+		ctdb_db->hot_keys[i].count = count;
 		goto sort_keys;
 	}
 
@@ -855,12 +855,14 @@ ctdb_update_db_stat_hot_keys(struct ctdb_db_context *ctdb_db, TDB_DATA key,
 		id = 0;
 	}
 
-	if (ctdb_db->statistics.hot_keys[id].key.dptr != NULL) {
-		talloc_free(ctdb_db->statistics.hot_keys[id].key.dptr);
+	if (ctdb_db->hot_keys[id].key.dptr != NULL) {
+		talloc_free(ctdb_db->hot_keys[id].key.dptr);
 	}
-	ctdb_db->statistics.hot_keys[id].key.dsize = key.dsize;
-	ctdb_db->statistics.hot_keys[id].key.dptr  = talloc_memdup(ctdb_db, key.dptr, key.dsize);
-	ctdb_db->statistics.hot_keys[id].count = count;
+	ctdb_db->hot_keys[id].key.dsize = key.dsize;
+	ctdb_db->hot_keys[id].key.dptr = talloc_memdup(ctdb_db,
+						       key.dptr,
+						       key.dsize);
+	ctdb_db->hot_keys[id].count = count;
 
 	keystr = hex_encode_talloc(ctdb_db,
 				   (unsigned char *)key.dptr, key.dsize);
@@ -871,17 +873,17 @@ ctdb_update_db_stat_hot_keys(struct ctdb_db_context *ctdb_db, TDB_DATA key,
 
 sort_keys:
 	for (i = 1; i < MAX_HOT_KEYS; i++) {
-		if (ctdb_db->statistics.hot_keys[i].count == 0) {
+		if (ctdb_db->hot_keys[i].count == 0) {
 			continue;
 		}
-		if (ctdb_db->statistics.hot_keys[i].count < ctdb_db->statistics.hot_keys[0].count) {
-			count = ctdb_db->statistics.hot_keys[i].count;
-			ctdb_db->statistics.hot_keys[i].count = ctdb_db->statistics.hot_keys[0].count;
-			ctdb_db->statistics.hot_keys[0].count = count;
-
-			key = ctdb_db->statistics.hot_keys[i].key;
-			ctdb_db->statistics.hot_keys[i].key = ctdb_db->statistics.hot_keys[0].key;
-			ctdb_db->statistics.hot_keys[0].key = key;
+		if (ctdb_db->hot_keys[i].count < ctdb_db->hot_keys[0].count) {
+			count = ctdb_db->hot_keys[i].count;
+			ctdb_db->hot_keys[i].count = ctdb_db->hot_keys[0].count;
+			ctdb_db->hot_keys[0].count = count;
+
+			key = ctdb_db->hot_keys[i].key;
+			ctdb_db->hot_keys[i].key = ctdb_db->hot_keys[0].key;
+			ctdb_db->hot_keys[0].key = key;
 		}
 	}
 }
diff --git a/ctdb/server/ctdb_ltdb_server.c b/ctdb/server/ctdb_ltdb_server.c
index ce3569fe7b1..4b14a6a2a16 100644
--- a/ctdb/server/ctdb_ltdb_server.c
+++ b/ctdb/server/ctdb_ltdb_server.c
@@ -1597,13 +1597,14 @@ int ctdb_set_db_sticky(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_d
 
 void ctdb_db_statistics_reset(struct ctdb_db_context *ctdb_db)
 {
-	struct ctdb_db_statistics_old *s = &ctdb_db->statistics;
 	int i;
 
 	for (i=0; i<MAX_HOT_KEYS; i++) {
-		if (s->hot_keys[i].key.dsize > 0) {
-			talloc_free(s->hot_keys[i].key.dptr);
+		if (ctdb_db->hot_keys[i].key.dsize > 0) {
+			TALLOC_FREE(ctdb_db->hot_keys[i].key.dptr);
+			ctdb_db->hot_keys[i].key.dsize = 0;
 		}
+		ctdb_db->hot_keys[i].count = 0;
 	}
 
 	ZERO_STRUCT(ctdb_db->statistics);
@@ -1627,7 +1628,13 @@ int32_t ctdb_control_get_db_statistics(struct ctdb_context *ctdb,
 
 	len = offsetof(struct ctdb_db_statistics_old, hot_keys_wire);
 	for (i = 0; i < MAX_HOT_KEYS; i++) {
-		len += ctdb_db->statistics.hot_keys[i].key.dsize;
+		struct ctdb_db_statistics_old *s = &ctdb_db->statistics;
+
+		s->hot_keys[i].key.dsize = ctdb_db->hot_keys[i].key.dsize;
+		s->hot_keys[i].key.dptr = ctdb_db->hot_keys[i].key.dptr;
+		s->hot_keys[i].count = ctdb_db->hot_keys[i].count;
+
+		len += s->hot_keys[i].key.dsize;
 	}
 
 	stats = talloc_size(outdata, len);