@cryptotaxi247 / netdata-1 / commits / b1465aa58

When unregistering an ephemeral host, delete its chart labels (#16486)

* When unregistering an ephemeral host, delete its chart labels * Fix memory leak in case of query preparation or bind failure * Add check to handle CID 410125 Dereference null return value

Stelios Fragkakis committed Nov 29, 2023 at 15:57 UTC b1465aa5895326c4100e3b535364ca78f91b6f71

4 files changed +106 -12

collectors/plugins.d/pluginsd_parser.c

+2 -1

@@ -1422,7 +1422,8 @@ static inline PARSER_RC pluginsd_label(char **words, size_t num_words, PARSER *p

1422 int is_ephemeral = appconfig_test_boolean_value((char *) value);

1423 if (is_ephemeral) {

1424 RRDHOST *host = pluginsd_require_scope_host(parser, PLUGINSD_KEYWORD_LABEL);

1425 - rrdhost_option_set(host, RRDHOST_OPTION_EPHEMERAL_HOST);

1425 + if (likely(host))

1426 + rrdhost_option_set(host, RRDHOST_OPTION_EPHEMERAL_HOST);

1427 }

1428 }

1429

database/sqlite/sqlite_aclk.c

+18 -9

@@ -257,30 +257,38 @@ static void sql_unregister_node(char *machine_guid)

257 return;

258

259 rc = uuid_parse(machine_guid, host_uuid);

260 - freez(machine_guid);

261 - if (rc)

260 + if (rc) {

261 + freez(machine_guid);

262 return;

263 + }

264

265 sqlite3_stmt *res = NULL;

266

267 rc = sqlite3_prepare_v2(db_meta, "UPDATE node_instance SET node_id = NULL WHERE host_id = @host_id", -1, &res, 0);

268 if (unlikely(rc != SQLITE_OK)) {

268 - error_report("Failed to prepare statement remote node id for a host");

269 + error_report("Failed to prepare statement to remove the host node id");

270 + freez(machine_guid);

271 return;

272 }

273

274 rc = sqlite3_bind_blob(res, 1, &host_uuid, sizeof(host_uuid), SQLITE_STATIC);

275 if (unlikely(rc != SQLITE_OK)) {

274 - error_report("Failed to bind host_id parameter to remove node id");

275 - goto failed;

276 + error_report("Failed to bind host_id parameter to remove host node id");

277 + goto skip;

278 }

279 rc = sqlite3_step_monitored(res);

278 - if (unlikely(rc != SQLITE_DONE))

279 - error_report("Failed to execute command to remove node id");

280 + if (unlikely(rc != SQLITE_DONE)) {

281 + error_report("Failed to execute command to remove host node id");

282 + } else {

283 + // node: machine guid will be freed after processing

284 + metadata_delete_host_chart_labels(machine_guid);

285 + machine_guid = NULL;

286 + }

287

281 -failed:

288 +skip:

289 if (unlikely(sqlite3_finalize(res) != SQLITE_OK))

283 - error_report("Failed to finalize statement to remove node id");

290 + error_report("Failed to finalize statement to remove host node id");

291 + freez(machine_guid);

292 }

293

294

@@ -448,6 +456,7 @@ static void aclk_synchronization(void *arg __maybe_unused)

456 break;

457 case ACLK_DATABASE_NODE_UNREGISTER:

458 sql_unregister_node(cmd.param[0]);

459 +

460 break;

461 // ALERTS

462 case ACLK_DATABASE_PUSH_ALERT_CONFIG:

database/sqlite/sqlite_metadata.c

+85 -2

@@ -80,6 +80,7 @@ enum metadata_opcode {

80 METADATA_ADD_HOST_INFO,

81 METADATA_SCAN_HOSTS,

82 METADATA_LOAD_HOST_CONTEXT,

83 + METADATA_DELETE_HOST_CHART_LABELS,

84 METADATA_MAINTENANCE,

85 METADATA_SYNC_SHUTDOWN,

86 METADATA_UNITTEST,

@@ -119,6 +120,8 @@ struct metadata_wc {

120 #define metadata_flag_set(target_flags, flag) __atomic_or_fetch(&((target_flags)->flags), (flag), __ATOMIC_SEQ_CST)

121 #define metadata_flag_clear(target_flags, flag) __atomic_and_fetch(&((target_flags)->flags), ~(flag), __ATOMIC_SEQ_CST)

122

123 +struct metadata_wc metasync_worker = {.loop = NULL};

124 +

125 //

126 // For unittest

127 //

@@ -138,6 +141,33 @@ struct query_build {

141 char uuid_str[UUID_STR_LEN];

142 };

143

144 +#define SQL_DELETE_CHART_LABELS_BY_HOST \

145 + "DELETE FROM chart_label WHERE chart_id in (SELECT chart_id FROM chart WHERE host_id = @host_id)"

146 +

147 +static void delete_host_chart_labels(uuid_t *host_uuid)

148 +{

149 + sqlite3_stmt *res = NULL;

150 +

151 + int rc = sqlite3_prepare_v2(db_meta, SQL_DELETE_CHART_LABELS_BY_HOST, -1, &res, 0);

152 + if (unlikely(rc != SQLITE_OK)) {

153 + error_report("Failed to prepare statement to delete chart labels by host");

154 + return;

155 + }

156 +

157 + rc = sqlite3_bind_blob(res, 1, host_uuid, sizeof(*host_uuid), SQLITE_STATIC);

158 + if (unlikely(rc != SQLITE_OK)) {

159 + error_report("Failed to bind host_id parameter to host chart labels");

160 + goto failed;

161 + }

162 + rc = sqlite3_step_monitored(res);

163 + if (unlikely(rc != SQLITE_DONE))

164 + error_report("Failed to execute command to remove host chart labels");

165 +

166 +failed:

167 + if (unlikely(sqlite3_finalize(res) != SQLITE_OK))

168 + error_report("Failed to finalize statement to remove host chart labels");

169 +}

170 +

171 static int host_label_store_to_sql_callback(const char *name, const char *value, RRDLABEL_SRC ls, void *data) {

172 struct query_build *lb = data;

173 if (unlikely(!lb->count))

@@ -1173,6 +1203,7 @@ void run_metadata_cleanup(struct metadata_wc *wc)

1203 struct scan_metadata_payload {

1204 uv_work_t request;

1205 struct metadata_wc *wc;

1206 + void *data;

1207 BUFFER *work_buffer;

1208 uint32_t max_count;

1209 };

@@ -1411,6 +1442,11 @@ static void store_host_and_system_info(RRDHOST *host, size_t *query_counter)

1442 }

1443 }

1444

1445 +struct host_chart_label_cleanup {

1446 + Pvoid_t JudyL;

1447 + Word_t count;

1448 +};

1449 +

1450 // Worker thread to scan hosts for pending metadata to store

1451 static void start_metadata_hosts(uv_work_t *req __maybe_unused)

1452 {

@@ -1427,6 +1463,29 @@ static void start_metadata_hosts(uv_work_t *req __maybe_unused)

1463 internal_error(true, "METADATA: checking all hosts...");

1464 usec_t started_ut = now_monotonic_usec(); (void)started_ut;

1465

1466 + struct host_chart_label_cleanup *cl_cleanup_data = data->data;

1467 +

1468 + if (cl_cleanup_data) {

1469 + Word_t Index = 0;

1470 + bool first = true;

1471 + Pvoid_t *PValue;

1472 + while ((PValue = JudyLFirstThenNext(cl_cleanup_data->JudyL, &Index, &first))) {

1473 + char *machine_guid = *PValue;

1474 +

1475 + host = rrdhost_find_by_guid(machine_guid);

1476 + if (unlikely(host))

1477 + continue;

1478 +

1479 + uuid_t host_uuid;

1480 + uuid_parse(machine_guid, host_uuid);

1481 + delete_host_chart_labels(&host_uuid);

1482 +

1483 + freez(machine_guid);

1484 + }

1485 + JudyLFreeArray(&cl_cleanup_data->JudyL, PJE0);

1486 + freez(cl_cleanup_data);

1487 + }

1488 +

1489 bool run_again = false;

1490 worker_is_busy(UV_EVENT_METADATA_STORE);

1491

@@ -1568,6 +1627,7 @@ static void metadata_event_loop(void *arg)

1627 completion_mark_complete(&wc->start_stop_complete);

1628 BUFFER *work_buffer = buffer_create(1024, &netdata_buffers_statistics.buffers_sqlite);

1629 struct scan_metadata_payload *data;

1630 + struct host_chart_label_cleanup *cl_cleanup_data = NULL;

1631

1632 while (shutdown == 0 || (wc->flags & METADATA_FLAG_PROCESSING)) {

1633 uuid_t *uuid;

@@ -1624,7 +1684,9 @@ static void metadata_event_loop(void *arg)

1684 data = mallocz(sizeof(*data));

1685 data->request.data = data;

1686 data->wc = wc;

1687 + data->data = cl_cleanup_data;

1688 data->work_buffer = work_buffer;

1689 + cl_cleanup_data = NULL;

1690

1691 if (unlikely(cmd.completion)) {

1692 data->max_count = 0; // 0 will process all pending updates

@@ -1640,6 +1702,7 @@ static void metadata_event_loop(void *arg)

1702 after_metadata_hosts))) {

1703 // Failed to launch worker -- let the event loop handle completion

1704 cmd.completion = wc->scan_complete;

1705 + cl_cleanup_data = data->data;

1706 freez(data);

1707 metadata_flag_clear(wc, METADATA_FLAG_PROCESSING);

1708 }

@@ -1656,6 +1719,15 @@ static void metadata_event_loop(void *arg)

1719 after_start_host_load_context))) {

1720 freez(data);

1721 }

1722 + break;

1723 + case METADATA_DELETE_HOST_CHART_LABELS:;

1724 + if (!cl_cleanup_data)

1725 + cl_cleanup_data = callocz(1,sizeof(*cl_cleanup_data));

1726 +

1727 + Pvoid_t *PValue = JudyLIns(&cl_cleanup_data->JudyL, (Word_t) ++cl_cleanup_data->count, PJE0);

1728 + if (PValue)

1729 + *PValue = (void *) cmd.param[0];

1730 +

1731 break;

1732 case METADATA_UNITTEST:;

1733 struct thread_unittest *tu = (struct thread_unittest *) cmd.param[0];

@@ -1702,8 +1774,6 @@ error_after_loop_init:

1774 worker_unregister();

1775 }

1776

1705 -struct metadata_wc metasync_worker = {.loop = NULL};

1706 -

1777 void metadata_sync_shutdown(void)

1778 {

1779 completion_init(&metasync_worker.start_stop_complete);

@@ -1828,6 +1898,19 @@ void metadata_queue_load_host_context(RRDHOST *host)

1898 nd_log(NDLS_DAEMON, NDLP_DEBUG, "Queued command to load host contexts");

1899 }

1900

1901 +void metadata_delete_host_chart_labels(char *machine_guid)

1902 +{

1903 + if (unlikely(!metasync_worker.loop)) {

1904 + freez(machine_guid);

1905 + return;

1906 + }

1907 +

1908 + // Node machine guid is already strdup-ed

1909 + queue_metadata_cmd(METADATA_DELETE_HOST_CHART_LABELS, machine_guid, NULL);

1910 + nd_log(NDLS_DAEMON, NDLP_DEBUG, "Queued command delete chart labels for host %s", machine_guid);

1911 +}

1912 +

1913 +

1914 //

1915 // unitests

1916 //

database/sqlite/sqlite_metadata.h

@@ -17,6 +17,7 @@ void metaqueue_host_update_info(RRDHOST *host);

17 void metaqueue_ml_load_models(RRDDIM *rd);

18 void migrate_localhost(uuid_t *host_uuid);

19 void metadata_queue_load_host_context(RRDHOST *host);

20 +void metadata_delete_host_chart_labels(char *machine_guid);

21 void vacuum_database(sqlite3 *database, const char *db_alias, int threshold, int vacuum_pc);

23 // UNIT TEST