@cryptotaxi247 / netdata / commits / 15eb93d1a

Improve hosts load on agent startup (#22446)

* perf(sqlite_aclk): JOIN + prepared statements for archived-hosts startup - SQL_FETCH_ALL_HOSTS: two correlated subqueries (host_label, node_instance) rewritten as two LEFT JOINs — one planned lookup instead of 2N extra index probes per startup. Dropped memory_mode and health_enabled columns (selected but not consumed). - Both sqlite3_exec_monitored sites in aclk_synchronization_init use prepared statements + sqlite3_column_* accessors; uuid columns go through sqlite3_column_uuid_copy. - Removed create_host_callback, aclk_config_parameters, the IDX_* enum; added load_archived_host_from_row() helper. * fix(sqlite_aclk): improve handling of NULL for update_every and enhance error logging * address review * Address review comments * review: log on UUID copy failures during archived-host load The two row-skip paths in aclk_synchronization_init (host load and node_instance loop) now log column type and byte count when sqlite3_column_uuid_copy() rejects the blob, so DB corruption surfaces instead of silently dropping hosts or ACLK configs. * refactor(sqlite_aclk): rename timezone variable to host_tz for clarity

Stelios Fragkakis committed May 13, 2026 at 10:06 UTC 15eb93d1ad5c18e94975b00fc768630e804e4f0e
1 file changed +164 -97
src/database/sqlite/sqlite_aclk.c
+164 -97
@@ -72,87 +72,109 @@ static bool aclk_database_enq_cmd(cmd_data_t *cmd, bool wait_on_full)
72 return added;
73 }
74
75 -enum {
76 - IDX_HOST_ID,
77 - IDX_HOSTNAME,
78 - IDX_REGISTRY,
79 - IDX_UPDATE_EVERY,
80 - IDX_OS,
81 - IDX_TIMEZONE,
82 - IDX_HOPS,
83 - IDX_MEMORY_MODE,
84 - IDX_ABBREV_TIMEZONE,
85 - IDX_UTC_OFFSET,
86 - IDX_PROGRAM_NAME,
87 - IDX_PROGRAM_VERSION,
88 - IDX_ENTRIES,
89 - IDX_HEALTH_ENABLED,
90 - IDX_LAST_CONNECTED,
91 - IDX_IS_EPHEMERAL,
92 - IDX_IS_REGISTERED,
93 -};
94 -
75 struct children {
76 int vnodes;
77 int normal;
78 };
79
100 -static int create_host_callback(void *data, int argc, char **argv, char **column)
80 +// Column indices for SQL_FETCH_ALL_HOSTS — keep in lock-step with the SELECT list.
81 +enum {
82 + COL_FETCH_HOST_ID = 0,
83 + COL_FETCH_HOSTNAME,
84 + COL_FETCH_REGISTRY,
85 + COL_FETCH_UPDATE_EVERY,
86 + COL_FETCH_OS,
87 + COL_FETCH_TIMEZONE,
88 + COL_FETCH_HOPS,
89 + COL_FETCH_ABBREV_TIMEZONE,
90 + COL_FETCH_UTC_OFFSET,
91 + COL_FETCH_PROGRAM_NAME,
92 + COL_FETCH_PROGRAM_VERSION,
93 + COL_FETCH_ENTRIES,
94 + COL_FETCH_LAST_CONNECTED,
95 + COL_FETCH_IS_EPHEMERAL,
96 + COL_FETCH_IS_REGISTERED,
97 +};
98 +
99 +// Materialise one archived host row from SQL_FETCH_ALL_HOSTS into rrdhost_root_index.
100 +// Returns the host (or NULL if creation skipped/failed) so the caller can update counters.
101 +static RRDHOST *load_archived_host_from_row(sqlite3_stmt *res)
102 {
102 - struct children *node_data = data;
103 - UNUSED(argc);
104 - UNUSED(column);
103 + // The COL_FETCH_* enum is in lock-step with SQL_FETCH_ALL_HOSTS' SELECT list.
104 + // Catch drift early in debug builds; release builds compile this out.
105 + internal_fatal(sqlite3_column_count(res) != COL_FETCH_IS_REGISTERED + 1,
106 + "SQL_FETCH_ALL_HOSTS column count (%d) does not match COL_FETCH_* enum (%d)",
107 + sqlite3_column_count(res), COL_FETCH_IS_REGISTERED + 1);
108
106 - time_t last_connected =
107 - (time_t)(argv[IDX_LAST_CONNECTED] ? str2uint64_t(argv[IDX_LAST_CONNECTED], NULL) : 0);
109 + nd_uuid_t host_uuid;
110 + if (!sqlite3_column_uuid_copy(res, COL_FETCH_HOST_ID, host_uuid)) {
111 + nd_log_daemon(
112 + NDLP_ERR,
113 + "Skipping archived host: host_id column is not a valid 16-byte UUID blob (type=%d, bytes=%d). Possible DB corruption.",
114 + sqlite3_column_type(res, COL_FETCH_HOST_ID),
115 + sqlite3_column_bytes(res, COL_FETCH_HOST_ID));
116 + return NULL;
117 + }
118
119 + char guid[UUID_STR_LEN];
120 + uuid_unparse_lower(host_uuid, guid);
121 +
122 + const char *hostname = (const char *)sqlite3_column_text(res, COL_FETCH_HOSTNAME);
123 + const char *registry = (const char *)sqlite3_column_text(res, COL_FETCH_REGISTRY);
124 + const char *os = (const char *)sqlite3_column_text(res, COL_FETCH_OS);
125 + const char *host_tz = (const char *)sqlite3_column_text(res, COL_FETCH_TIMEZONE);
126 + const char *abbrev_tz = (const char *)sqlite3_column_text(res, COL_FETCH_ABBREV_TIMEZONE);
127 + const char *prog_name = (const char *)sqlite3_column_text(res, COL_FETCH_PROGRAM_NAME);
128 + const char *prog_version = (const char *)sqlite3_column_text(res, COL_FETCH_PROGRAM_VERSION);
129 + int hops = sqlite3_column_int(res, COL_FETCH_HOPS);
130 + int utc_offset = sqlite3_column_int(res, COL_FETCH_UTC_OFFSET);
131 + int entries = sqlite3_column_int(res, COL_FETCH_ENTRIES);
132 + // update_every defaults to 1 only when the column is SQL NULL — preserves
133 + // the pre-refactor `argv[i] ? str2i(argv[i]) : 1` fallback exactly. A
134 + // stored 0 stays 0 (matches the original str2i path).
135 + int update_every = (sqlite3_column_type(res, COL_FETCH_UPDATE_EVERY) == SQLITE_NULL)
136 + ? 1
137 + : sqlite3_column_int(res, COL_FETCH_UPDATE_EVERY);
138 + int64_t last_connected_db = sqlite3_column_int64(res, COL_FETCH_LAST_CONNECTED);
139 + int is_ephemeral = sqlite3_column_int(res, COL_FETCH_IS_EPHEMERAL);
140 + int is_registered = sqlite3_column_int(res, COL_FETCH_IS_REGISTERED);
141 +
142 + time_t last_connected = (time_t)last_connected_db;
143 if (!last_connected)
144 last_connected = now_realtime_sec();
145
146 time_t age = now_realtime_sec() - last_connected;
113 - int is_ephemeral = 0;
114 - int is_registered = 0;
115 -
116 - if (argv[IDX_IS_EPHEMERAL])
117 - is_ephemeral = str2i(argv[IDX_IS_EPHEMERAL]);
118 -
119 - if (argv[IDX_IS_REGISTERED])
120 - is_registered = str2i(argv[IDX_IS_REGISTERED]);
147
122 - char guid[UUID_STR_LEN];
123 - uuid_unparse_lower(*(nd_uuid_t *)argv[IDX_HOST_ID], guid);
124 -
125 - if (is_ephemeral && ((!is_registered && last_connected == 1) || (rrdhost_free_ephemeral_time_s && age > rrdhost_free_ephemeral_time_s))) {
148 + if (is_ephemeral && ((!is_registered && last_connected == 1) ||
149 + (rrdhost_free_ephemeral_time_s && age > rrdhost_free_ephemeral_time_s))) {
150 netdata_log_info(
151 "%s ephemeral hostname \"%s\" with GUID \"%s\", age = %ld seconds (limit %ld seconds)",
152 is_registered ? "Loading registered" : "Skipping unregistered",
129 - (const char *)argv[IDX_HOSTNAME],
153 + hostname,
154 guid,
155 age,
156 rrdhost_free_ephemeral_time_s);
157
158 if (!is_registered)
135 - goto done;
159 + return NULL;
160 }
161
162 struct rrdhost_system_info *system_info = rrdhost_system_info_create();
139 -
140 - rrdhost_system_info_hops_set(system_info, (int16_t)str2i((const char *) argv[IDX_HOPS]));
141 -
142 - sql_build_host_system_info((nd_uuid_t *)argv[IDX_HOST_ID], system_info);
163 + rrdhost_system_info_hops_set(system_info, (int16_t)hops);
164 + sql_build_host_system_info(&host_uuid, system_info);
165
166 RRDHOST *host = rrdhost_find_or_create(
145 - (const char *)argv[IDX_HOSTNAME],
146 - (const char *)argv[IDX_REGISTRY],
167 + hostname,
168 + registry,
169 guid,
148 - (const char *)argv[IDX_OS],
149 - (const char *)argv[IDX_TIMEZONE],
150 - (const char *)argv[IDX_ABBREV_TIMEZONE],
151 - (int32_t)(argv[IDX_UTC_OFFSET] ? str2uint32_t(argv[IDX_UTC_OFFSET], NULL) : 0),
152 - (const char *)(argv[IDX_PROGRAM_NAME] ? argv[IDX_PROGRAM_NAME] : "unknown"),
153 - (const char *)(argv[IDX_PROGRAM_VERSION] ? argv[IDX_PROGRAM_VERSION] : "unknown"),
154 - argv[IDX_UPDATE_EVERY] ? str2i(argv[IDX_UPDATE_EVERY]) : 1,
155 - argv[IDX_ENTRIES] ? str2i(argv[IDX_ENTRIES]) : 0,
170 + os,
171 + host_tz,
172 + abbrev_tz,
173 + (int32_t)utc_offset,
174 + prog_name ? prog_name : "unknown",
175 + prog_version ? prog_version : "unknown",
176 + update_every,
177 + entries,
178 default_rrd_memory_mode,
179 0, // health
180 0, // rrdpush enabled
@@ -168,24 +190,18 @@ static int create_host_callback(void *data, int argc, char **argv, char **column
190 rrdhost_system_info_free(system_info);
191
192 if (unlikely(!host))
171 - return 0;
193 + return NULL;
194
173 - if (is_ephemeral)
195 + if (is_ephemeral) {
196 rrdhost_option_set(host, RRDHOST_OPTION_EPHEMERAL_HOST);
175 -
176 - if (is_ephemeral)
197 host->stream.rcv.status.last_disconnected = now_realtime_sec();
198 + }
199
179 - host->rrdlabels = sql_load_host_labels((nd_uuid_t *)argv[IDX_HOST_ID]);
200 + host->rrdlabels = sql_load_host_labels(&host_uuid);
201 host->stream.snd.status.last_connected = last_connected;
202
203 pulse_host_status(host, 0, 0); // this will detect the receiver status
204
184 - if (IS_VIRTUAL_HOST_OS(host))
185 - node_data->vnodes++;
186 - else
187 - node_data->normal++;
188 -
205 #ifdef NETDATA_INTERNAL_CHECKS
206 char node_str[UUID_STR_LEN] = "<none>";
207 if (likely(!UUIDiszero(host->node_id)))
@@ -194,8 +210,7 @@ static int create_host_callback(void *data, int argc, char **argv, char **column
210 rrdhost_hostname(host), host->machine_guid, node_str, is_ephemeral);
211 #endif
212
197 -done:
198 - return 0;
213 + return host;
214 }
215
216
@@ -285,17 +300,6 @@ skip:
300 freez(machine_guid);
301 }
302
288 -static int aclk_config_parameters(void *data __maybe_unused, int argc __maybe_unused, char **argv, char **column __maybe_unused)
289 -{
290 - char uuid_str[UUID_STR_LEN];
291 - uuid_unparse_lower(*((nd_uuid_t *) argv[0]), uuid_str);
292 -
293 - RRDHOST *host = rrdhost_find_by_guid(uuid_str);
294 - if (host != localhost)
295 - create_aclk_config(host, (nd_uuid_t *)argv[0], (nd_uuid_t *)argv[1]);
296 - return 0;
297 -}
298 -
303 struct judy_list_t {
304 Pvoid_t JudyL;
305 Word_t count;
@@ -1015,14 +1019,26 @@ void create_aclk_config(RRDHOST *host, nd_uuid_t *host_uuid __maybe_unused, nd_u
1019 }
1020 }
1021
1022 +// Replaces two correlated subqueries (host_label, node_instance) with LEFT JOINs
1023 +// so the planner does the lookup once per row instead of 2N extra index probes.
1024 +// memory_mode and health_enabled are intentionally omitted — they were SELECTed
1025 +// in the previous shape but never read by the consumer.
1026 +//
1027 +// Both LEFT JOINs are guaranteed to match at most one row per host by the
1028 +// schema (sqlite_metadata.c database_config[]): host_label has
1029 +// PRIMARY KEY (host_id, label_key), and node_instance has host_id PRIMARY KEY.
1030 +// Row multiplication is therefore impossible here without a SQLite invariant
1031 +// violation; no DISTINCT / GROUP BY / EXISTS wrapper needed.
1032 #define SQL_FETCH_ALL_HOSTS \
1019 - "SELECT host_id, hostname, registry_hostname, update_every, os, " \
1020 - "timezone, hops, memory_mode, abbrev_timezone, utc_offset, program_name, " \
1021 - "program_version, entries, health_enabled, last_connected, " \
1022 - "(SELECT CASE WHEN hl.label_value = 'true' THEN 1 ELSE 0 END FROM " \
1023 - "host_label hl WHERE hl.host_id = h.host_id AND hl.label_key = '_is_ephemeral'), " \
1024 - "(SELECT CASE WHEN ni.node_id is NULL THEN 0 ELSE 1 END FROM " \
1025 - "node_instance ni WHERE ni.host_id = h.host_id) FROM host h WHERE hops > 0"
1033 + "SELECT h.host_id, h.hostname, h.registry_hostname, h.update_every, h.os, " \
1034 + "h.timezone, h.hops, h.abbrev_timezone, h.utc_offset, h.program_name, " \
1035 + "h.program_version, h.entries, h.last_connected, " \
1036 + "CASE WHEN hl.label_value = 'true' THEN 1 ELSE 0 END, " \
1037 + "CASE WHEN ni.node_id IS NULL THEN 0 ELSE 1 END " \
1038 + "FROM host h " \
1039 + "LEFT JOIN host_label hl ON hl.host_id = h.host_id AND hl.label_key = '_is_ephemeral' " \
1040 + "LEFT JOIN node_instance ni ON ni.host_id = h.host_id " \
1041 + "WHERE h.hops > 0"
1042
1043 #define SQL_FETCH_ALL_INSTANCES \
1044 "SELECT ni.host_id, ni.node_id FROM host h, node_instance ni " \
@@ -1033,18 +1049,33 @@ uv_sem_t ctx_sem;
1049
1050 void aclk_synchronization_init(void)
1051 {
1036 - char *err_msg = NULL;
1037 - int rc;
1038 -
1052 nd_log_daemon(NDLP_INFO, "Creating archived hosts");
1040 - struct children node_data = { 0, 0};
1041 -
1042 - rc = sqlite3_exec_monitored(db_meta, SQL_FETCH_ALL_HOSTS, create_host_callback, &node_data, &err_msg);
1053 + struct children node_data = { 0, 0 };
1054
1044 - if (rc != SQLITE_OK) {
1045 - nd_log_daemon(NDLP_ERR, "SQLite error when loading archived hosts, rc = %d (%s)", rc, err_msg);
1046 - sqlite3_free(err_msg);
1055 + sqlite3_stmt *res = NULL;
1056 + if (PREPARE_STATEMENT(db_meta, SQL_FETCH_ALL_HOSTS, &res)) {
1057 + int step_rc;
1058 + while ((step_rc = sqlite3_step_monitored(res)) == SQLITE_ROW) {
1059 + RRDHOST *host = load_archived_host_from_row(res);
1060 + if (!host)
1061 + continue;
1062 + if (IS_VIRTUAL_HOST_OS(host))
1063 + node_data.vnodes++;
1064 + else
1065 + node_data.normal++;
1066 + }
1067 + if (step_rc != SQLITE_DONE)
1068 + nd_log_daemon(
1069 + NDLP_ERR,
1070 + "SQLite error while loading archived hosts, rc = %d (%s); load may be partial",
1071 + step_rc,
1072 + sqlite3_errmsg(db_meta));
1073 + SQLITE_FINALIZE(res);
1074 }
1075 + else
1076 + nd_log_daemon(NDLP_ERR,
1077 + "SQLite error when preparing statement to load archived hosts: %s",
1078 + sqlite3_errmsg(db_meta));
1079
1080 nd_log_daemon(
1081 NDLP_INFO,
@@ -1065,12 +1096,48 @@ void aclk_synchronization_init(void)
1096 sem_init = false;
1097 }
1098
1068 - rc = sqlite3_exec_monitored(db_meta, SQL_FETCH_ALL_INSTANCES, aclk_config_parameters, NULL, &err_msg);
1099 + sqlite3_stmt *res_inst = NULL;
1100 + if (PREPARE_STATEMENT(db_meta, SQL_FETCH_ALL_INSTANCES, &res_inst)) {
1101 + int step_rc;
1102 + while ((step_rc = sqlite3_step_monitored(res_inst)) == SQLITE_ROW) {
1103 + nd_uuid_t host_uuid, node_uuid;
1104 + if (!sqlite3_column_uuid_copy(res_inst, 0, host_uuid)) {
1105 + nd_log_daemon(
1106 + NDLP_ERR,
1107 + "Skipping node_instance row: host_id (col 0) is not a valid 16-byte UUID blob (type=%d, bytes=%d). ACLK config not configured for this host.",
1108 + sqlite3_column_type(res_inst, 0),
1109 + sqlite3_column_bytes(res_inst, 0));
1110 + continue;
1111 + }
1112 + if (!sqlite3_column_uuid_copy(res_inst, 1, node_uuid)) {
1113 + nd_log_daemon(
1114 + NDLP_ERR,
1115 + "Skipping node_instance row: node_id (col 1) is not a valid 16-byte UUID blob (type=%d, bytes=%d). ACLK config not configured for this host.",
1116 + sqlite3_column_type(res_inst, 1),
1117 + sqlite3_column_bytes(res_inst, 1));
1118 + continue;
1119 + }
1120
1070 - if (rc != SQLITE_OK) {
1071 - nd_log_daemon(NDLP_ERR, "SQLite error when configuring host ACLK synchonization parameters, rc = %d (%s)", rc, err_msg);
1072 - sqlite3_free(err_msg);
1121 + char uuid_str[UUID_STR_LEN];
1122 + uuid_unparse_lower(host_uuid, uuid_str);
1123 + RRDHOST *host = rrdhost_find_by_guid(uuid_str);
1124 + // create_aclk_config() already null-checks `host`, but the explicit
1125 + // guard makes the intent clear and skips the call for unknown GUIDs.
1126 + if (host && host != localhost)
1127 + create_aclk_config(host, &host_uuid, &node_uuid);
1128 + }
1129 + if (step_rc != SQLITE_DONE)
1130 + nd_log_daemon(
1131 + NDLP_ERR,
1132 + "SQLite error while configuring host ACLK synchronization parameters, rc = %d (%s); some configs may be missing",
1133 + step_rc,
1134 + sqlite3_errmsg(db_meta));
1135 + SQLITE_FINALIZE(res_inst);
1136 }
1137 + else
1138 + nd_log_daemon(NDLP_ERR,
1139 + "SQLite error when preparing statement to configure host ACLK synchronization parameters: %s",
1140 + sqlite3_errmsg(db_meta));
1141
1142 aclk_initialize_event_loop();
1143