@cryptotaxi247 / netdata-1 / commits / 932bbf3b4

Finding leaks No 2 (#19823)

* remove the RRDHOST hostname index * do not use flags for indexing hosts * add dictionary_garbage_collect() to dictionary_flush() * just destroy remaining hot/dirty pages on shutdown * fix dictionaries not adding dictionaries for garbage collect

Costa Tsaousis committed Mar 11, 2025 at 15:36 UTC 932bbf3b4e31eb83bf49201d5e833695d176380d

12 files changed +83 -121

src/daemon/commands.c

+1 -1

@@ -410,7 +410,7 @@ static cmd_status_t cmd_remove_stale_node_internal(char *args, char **message, b

410 RRDHOST *host = NULL;

411 host = rrdhost_find_by_guid(args);

412 if (!host)

413 - host = find_host_by_node_id(args);

413 + host = rrdhost_find_by_node_id(args);

414

415 if (!host) {

416 sqlite3_stmt *res = NULL;

src/daemon/daemon-shutdown.c

+5 -5

@@ -119,7 +119,7 @@ static void rrdeng_flush_everything_and_wait(bool wait_flush, bool wait_collecto

119

120 if(wait_collectors) {

121 size_t running = 1;

122 - size_t count = 10;

122 + size_t count = 50;

123 while (running && count) {

124 running = 0;

125 for (size_t tier = 0; tier < nd_profile.storage_tiers; tier++)

@@ -272,7 +272,7 @@ void netdata_cleanup_and_exit(EXIT_REASON reason, const char *action, const char

272 th[tier] = nd_thread_create("rrdeng-exit", NETDATA_THREAD_OPTION_JOINABLE, rrdeng_exit_background, multidb_ctx[tier]);

273

274 // flush anything remaining again - just in case

275 - rrdeng_flush_everything_and_wait(true, false, false);

275 + rrdeng_flush_everything_and_wait(true, true, false);

276

277 for (size_t tier = 0; tier < nd_profile.storage_tiers; tier++)

278 nd_thread_join(th[tier]);

@@ -334,11 +334,11 @@ void netdata_cleanup_and_exit(EXIT_REASON reason, const char *action, const char

334

335 // destroy the caches in reverse order (extent and open depend on main cache)

336 fprintf(stderr, "Destroying extent cache (PGC)...\n");

337 - pgc_destroy(extent_cache);

337 + pgc_destroy(extent_cache, false);

338 fprintf(stderr, "Destroying open cache (PGC)...\n");

339 - pgc_destroy(open_cache);

339 + pgc_destroy(open_cache, false);

340 fprintf(stderr, "Destroying main cache (PGC)...\n");

341 - pgc_destroy(main_cache);

341 + pgc_destroy(main_cache, false);

342

343 fprintf(stderr, "Destroying metrics registry (MRG)...\n");

344 size_t metrics_referenced = mrg_destroy(main_mrg);

src/database/contexts/rrdcontext.c

+2 -2

@@ -214,7 +214,7 @@ void rrdcontext_hub_checkpoint_command(void *ptr) {

214 return;

215 }

216

217 - RRDHOST *host = find_host_by_node_id(cmd->node_id);

217 + RRDHOST *host = rrdhost_find_by_node_id(cmd->node_id);

218 if(!host) {

219 nd_log(NDLS_DAEMON, NDLP_WARNING,

220 "RRDCONTEXT: received checkpoint command for claim id '%s', node id '%s', "

@@ -286,7 +286,7 @@ void rrdcontext_hub_stop_streaming_command(void *ptr) {

286 return;

287 }

288

289 - RRDHOST *host = find_host_by_node_id(cmd->node_id);

289 + RRDHOST *host = rrdhost_find_by_node_id(cmd->node_id);

290 if(!host) {

291 nd_log(NDLS_DAEMON, NDLP_WARNING,

292 "RRDCONTEXT: received stop streaming command for claim id '%s', node id '%s', "

src/database/engine/cache.c

+10 -3

@@ -1843,7 +1843,9 @@ static bool flush_pages(PGC *cache, size_t max_flushes, Word_t section, bool wai

1843

1844 // call the callback to save them

1845 // it may take some time, so let's release the lock

1846 - cache->config.pgc_save_dirty_cb(cache, array, pages, pages_added);

1846 + if(cache->config.pgc_save_dirty_cb)

1847 + cache->config.pgc_save_dirty_cb(cache, array, pages, pages_added);

1848 +

1849 flushes_so_far++;

1850

1851 __atomic_add_fetch(&cache->stats.flushes_completed, pages_added, __ATOMIC_RELAXED);

@@ -2086,7 +2088,12 @@ void pgc_flush_all_hot_and_dirty_pages(PGC *cache, Word_t section) {

2088 flush_pages(cache, 0, section, true, true);

2089 }

2090

2089 -void pgc_destroy(PGC *cache) {

2091 +void pgc_destroy(PGC *cache, bool flush) {

2092 + if(!flush) {

2093 + cache->config.pgc_save_init_cb = NULL;

2094 + cache->config.pgc_save_dirty_cb = NULL;

2095 + }

2096 +

2097 // convert all hot pages to dirty

2098 all_hot_pages_to_dirty(cache, PGC_SECTION_ALL);

2099

@@ -3064,7 +3071,7 @@ int pgc_unittest(void) {

3071 pgc_page_hot_set_end_time_s(cache, page3, 2001, 0);

3072 pgc_page_hot_to_dirty_and_release(cache, page3, false);

3073

3067 - pgc_destroy(cache);

3074 + pgc_destroy(cache, true);

3075

3076 #ifdef PGC_STRESS_TEST

3077 unittest_stress_test();

src/database/engine/cache.h

+1 -1

@@ -175,7 +175,7 @@ PGC *pgc_create(const char *name,

175 PGC_OPTIONS options, size_t partitions, size_t additional_bytes_per_page);

176

177 // destroy the cache

178 -void pgc_destroy(PGC *cache);

178 +void pgc_destroy(PGC *cache, bool flush);

179

180 #define PGC_SECTION_ALL ((Word_t)0)

181 void pgc_flush_dirty_pages(PGC *cache, Word_t section);

src/database/rrdhost.c

+44 -86

@@ -9,7 +9,7 @@

9 RRDHOST *localhost = NULL;

10 netdata_rwlock_t rrd_rwlock = NETDATA_RWLOCK_INITIALIZER;

12 -RRDHOST *find_host_by_node_id(char *node_id) {

12 +RRDHOST *rrdhost_find_by_node_id(char *node_id) {

14 ND_UUID node_uuid;

15 if (unlikely(!node_id || uuid_parse(node_id, node_uuid.uuid)))

@@ -27,11 +27,30 @@ RRDHOST *find_host_by_node_id(char *node_id) {

27 return ret;

28 }

30 +RRDHOST *rrdhost_find_by_hostname(const char *hostname) {

31 + if(strcmp(hostname, "localhost") == 0)

32 + return localhost;

33 +

34 + STRING *name = string_strdupz(hostname);

35 +

36 + RRDHOST *host, *ret = NULL;

37 + dfe_start_read(rrdhost_root_index, host) {

38 + if (host->hostname == name) {

39 + ret = host;

40 + break;

41 + }

42 + }

43 + dfe_done(host);

44 +

45 + string_freez(name);

46 +

47 + return ret;

48 +}

49 +

50 // ----------------------------------------------------------------------------

51 // RRDHOST indexes management

53 DICTIONARY *rrdhost_root_index = NULL;

34 -static DICTIONARY *rrdhost_root_index_hostname = NULL;

55 void rrdhost_init() {

56 if(unlikely(!rrdhost_root_index)) {

@@ -39,12 +58,6 @@ void rrdhost_init() {

58 DICT_OPTION_NAME_LINK_DONT_CLONE | DICT_OPTION_VALUE_LINK_DONT_CLONE | DICT_OPTION_DONT_OVERWRITE_VALUE,

59 &dictionary_stats_category_rrdhost, 0);

60 }

42 -

43 - if(unlikely(!rrdhost_root_index_hostname)) {

44 - rrdhost_root_index_hostname = dictionary_create_advanced(

45 - DICT_OPTION_NAME_LINK_DONT_CLONE | DICT_OPTION_VALUE_LINK_DONT_CLONE | DICT_OPTION_DONT_OVERWRITE_VALUE,

46 - &dictionary_stats_category_rrdhost, 0);

47 - }

61 }

63 RRDHOST_ACQUIRED *rrdhost_find_and_acquire(const char *machine_guid) {

@@ -77,91 +90,36 @@ inline RRDHOST *rrdhost_find_by_guid(const char *guid) {

90 }

92 static inline RRDHOST *rrdhost_index_add_by_guid(RRDHOST *host) {

80 - RRDHOST *ret_machine_guid = dictionary_set(rrdhost_root_index, host->machine_guid, host, sizeof(RRDHOST));

81 - if(ret_machine_guid == host)

82 - rrdhost_option_set(host, RRDHOST_OPTION_INDEXED_MACHINE_GUID);

83 - else {

84 - rrdhost_option_clear(host, RRDHOST_OPTION_INDEXED_MACHINE_GUID);

85 - nd_log(NDLS_DAEMON, NDLP_NOTICE,

86 - "RRDHOST: host with machine guid '%s' is already indexed. Not adding it again.",

87 - host->machine_guid);

88 - }

89 -

90 - return host;

93 + return dictionary_set(rrdhost_root_index, host->machine_guid, host, sizeof(RRDHOST));

94 }

96 static void rrdhost_index_del_by_guid(RRDHOST *host) {

94 - if(rrdhost_option_check(host, RRDHOST_OPTION_INDEXED_MACHINE_GUID)) {

95 - if(!dictionary_del(rrdhost_root_index, host->machine_guid))

97 + RRDHOST *t = rrdhost_find_by_guid(host->machine_guid);

98 + if(t == host) {

99 + if (!dictionary_del(rrdhost_root_index, host->machine_guid))

100 + nd_log(

101 + NDLS_DAEMON, NDLP_NOTICE,

102 + "RRDHOST: failed to delete machine guid '%s' from index",

103 + host->machine_guid);

104 + }

105 + else

106 nd_log(NDLS_DAEMON, NDLP_NOTICE,

97 - "RRDHOST: failed to delete machine guid '%s' from index",

107 + "RRDHOST: failed to delete machine guid '%s' from index, not found",

108 host->machine_guid);

99 -

100 - rrdhost_option_clear(host, RRDHOST_OPTION_INDEXED_MACHINE_GUID);

101 - }

102 -}

103 -

104 -// ----------------------------------------------------------------------------

105 -// RRDHOST index by hostname

106 -

107 -inline RRDHOST *rrdhost_find_by_hostname(const char *hostname) {

108 - if(unlikely(!strcmp(hostname, "localhost")))

109 - return localhost;

110 -

111 - RRDHOST *host = dictionary_get(rrdhost_root_index_hostname, hostname);

112 - return host;

113 -}

114 -

115 -static inline void rrdhost_index_del_hostname(RRDHOST *host) {

116 - if(unlikely(!host->hostname)) return;

117 -

118 - if(rrdhost_option_check(host, RRDHOST_OPTION_INDEXED_HOSTNAME)) {

119 - if(!dictionary_del(rrdhost_root_index_hostname, rrdhost_hostname(host)))

120 - nd_log(NDLS_DAEMON, NDLP_NOTICE,

121 - "RRDHOST: failed to delete hostname '%s' from index",

122 - rrdhost_hostname(host));

123 -

124 - rrdhost_option_clear(host, RRDHOST_OPTION_INDEXED_HOSTNAME);

125 - }

126 -}

127 -

128 -static inline RRDHOST *rrdhost_index_add_hostname(RRDHOST *host) {

129 - if(!host->hostname) return host;

130 -

131 - RRDHOST *ret_hostname = dictionary_set(rrdhost_root_index_hostname, rrdhost_hostname(host), host, sizeof(RRDHOST));

132 - if(ret_hostname == host)

133 - rrdhost_option_set(host, RRDHOST_OPTION_INDEXED_HOSTNAME);

134 - else {

135 - // have the same hostname, but it's not the same host

136 - // keep the new one only if the old one is orphan or archived

137 - if (rrdhost_flag_check(ret_hostname, RRDHOST_FLAG_ORPHAN) || rrdhost_flag_check(ret_hostname, RRDHOST_FLAG_ARCHIVED)) {

138 - rrdhost_index_del_hostname(ret_hostname);

139 - rrdhost_index_add_hostname(host);

140 - }

141 - else

142 - rrdhost_option_clear(host, RRDHOST_OPTION_INDEXED_HOSTNAME);

143 - }

144 -

145 - return host;

109 }

110

111 // ----------------------------------------------------------------------------

112 // RRDHOST - internal helpers

113

151 -static inline void rrdhost_init_hostname(RRDHOST *host, const char *hostname, bool add_to_index) {

114 +static inline void rrdhost_init_hostname(RRDHOST *host, const char *hostname) {

115 if(unlikely(hostname && !*hostname)) hostname = NULL;

116

117 if(host->hostname && hostname && !strcmp(rrdhost_hostname(host), hostname))

118 return;

119

157 - rrdhost_index_del_hostname(host);

158 -

120 STRING *old = host->hostname;

121 host->hostname = string_strdupz(hostname?hostname:"localhost");

122 string_freez(old);

162 -

163 - if(add_to_index)

164 - rrdhost_index_add_hostname(host);

123 }

124

125 static inline void rrdhost_init_os(RRDHOST *host, const char *os) {

@@ -349,7 +307,7 @@ RRDHOST *rrdhost_create(

307 prog_name,

308 prog_version);

309

352 - rrdhost_init_hostname(host, hostname, false);

310 + rrdhost_init_hostname(host, hostname);

311

312 host->rrd_history_entries = align_entries_to_pagesize(memory_mode, entries);

313 host->health.enabled = ((memory_mode == RRD_DB_MODE_NONE)) ? false : health;

@@ -379,8 +337,7 @@ RRDHOST *rrdhost_create(

337 host->cache_dir = strdupz(netdata_configured_cache_dir);

338

339 // this is also needed for custom host variables - not only health

382 - if(!host->rrdvars)

383 - host->rrdvars = rrdvariables_create();

340 + host->rrdvars = rrdvariables_create();

341

342 if (likely(!uuid_parse(host->machine_guid, host->host_id.uuid)))

343 sql_load_node_id(host);

@@ -454,8 +411,6 @@ RRDHOST *rrdhost_create(

411 return NULL;

412 }

413

457 - rrdhost_index_add_hostname(host);

458 -

414 if(is_localhost)

415 DOUBLE_LINKED_LIST_PREPEND_ITEM_UNSAFE(localhost, host, prev, next);

416 else

@@ -560,9 +515,7 @@ static void rrdhost_update(RRDHOST *host

515 "Host '%s' has been renamed to '%s'. If this is not intentional it may mean multiple hosts are using the same machine_guid.",

516 rrdhost_hostname(host), hostname);

517

563 - rrdhost_init_hostname(host, hostname, true);

564 - } else {

565 - rrdhost_index_add_hostname(host);

518 + rrdhost_init_hostname(host, hostname);

519 }

520

521 if(strcmp(rrdhost_program_name(host), prog_name) != 0) {

@@ -814,7 +767,6 @@ void rrdhost_free___while_having_rrd_wrlock(RRDHOST *host) {

767 // ------------------------------------------------------------------------

768 // first remove it from the indexes, so that it will not be discoverable

769

817 - rrdhost_index_del_hostname(host);

770 rrdhost_index_del_by_guid(host);

771

772 if (host->prev)

@@ -868,9 +820,15 @@ void rrdhost_free_all(void) {

820

821 localhost = NULL;

822

871 - dictionary_destroy(rrdhost_root_index_hostname);

823 + RRDHOST *host;

824 + dfe_start_write(rrdhost_root_index, host) {

825 + fprintf(stderr, "RRDHOST: MACHINE_GUID '%s' is still in the dictionary!\n",

826 + host_dfe.name);

827 + }

828 + dfe_done(host);

829 +

830 + dictionary_garbage_collect(rrdhost_root_index);

831 dictionary_destroy(rrdhost_root_index);

873 - rrdhost_root_index_hostname = NULL;

832 rrdhost_root_index = NULL;

833

834 rrd_wrunlock();

src/database/rrdhost.h

+5 -9

@@ -94,17 +94,13 @@ typedef enum __attribute__ ((__packed__)) rrdhost_flags {

94 #define rrdhost_flag_set_and_clear(host, set, clear) atomic_flags_set_and_clear(&((host)->flags), set, clear)

96 typedef enum __attribute__ ((__packed__)) {

97 - // Indexing

98 - RRDHOST_OPTION_INDEXED_MACHINE_GUID = (1 << 0), // when set, we have indexed its machine guid

99 - RRDHOST_OPTION_INDEXED_HOSTNAME = (1 << 1), // when set, we have indexed its hostname

100 -

97 // Streaming configuration

102 - RRDHOST_OPTION_SENDER_ENABLED = (1 << 2), // set when the host is configured to send metrics to a parent

103 - RRDHOST_OPTION_REPLICATION = (1 << 3), // when set, we support replication for this host

98 + RRDHOST_OPTION_SENDER_ENABLED = (1 << 0), // set when the host is configured to send metrics to a parent

99 + RRDHOST_OPTION_REPLICATION = (1 << 1), // when set, we support replication for this host

100

101 // Other options

106 - RRDHOST_OPTION_VIRTUAL_HOST = (1 << 4), // when set, this host is a virtual one

107 - RRDHOST_OPTION_EPHEMERAL_HOST = (1 << 5), // when set, this host is an ephemeral one

102 + RRDHOST_OPTION_VIRTUAL_HOST = (1 << 2), // when set, this host is a virtual one

103 + RRDHOST_OPTION_EPHEMERAL_HOST = (1 << 3), // when set, this host is an ephemeral one

104 } RRDHOST_OPTIONS;

105

106 #define rrdhost_option_check(host, flag) ((host)->options & (flag))

@@ -388,7 +384,7 @@ void rrdhost_acquired_release(RRDHOST_ACQUIRED *rha);

384

385 RRDHOST *rrdhost_find_by_hostname(const char *hostname);

386 RRDHOST *rrdhost_find_by_guid(const char *guid);

391 -RRDHOST *find_host_by_node_id(char *node_id);

387 +RRDHOST *rrdhost_find_by_node_id(char *node_id);

388

389 #ifdef RRDHOST_INTERNALS

390 RRDHOST *rrdhost_create(

src/database/sqlite/sqlite_aclk_alert.c

+3 -3

@@ -742,7 +742,7 @@ void aclk_push_alert_config_event(char *node_id __maybe_unused, char *config_has

742 sqlite3_stmt *res = NULL;

743 struct aclk_sync_cfg_t *wc;

744

745 - RRDHOST *host = find_host_by_node_id(node_id);

745 + RRDHOST *host = rrdhost_find_by_node_id(node_id);

746

747 if (unlikely(!host || !(wc = host->aclk_config))) {

748 freez(config_hash);

@@ -1053,7 +1053,7 @@ void aclk_start_alert_streaming(char *node_id, uint64_t cloud_version)

1053 return;

1054

1055 struct aclk_sync_cfg_t *wc;

1056 - RRDHOST *host = find_host_by_node_id(node_id);

1056 + RRDHOST *host = rrdhost_find_by_node_id(node_id);

1057

1058 if (unlikely(!host || !(wc = host->aclk_config))) {

1059 nd_log(NDLS_ACCESS, NDLP_NOTICE, "ACLK STA [%s (N/A)]: Ignoring request to stream alert state changes, invalid node.", node_id);

@@ -1087,7 +1087,7 @@ void aclk_alert_version_check(char *node_id, char *claim_id, uint64_t cloud_vers

1087 }

1088

1089 struct aclk_sync_cfg_t *wc;

1090 - RRDHOST *host = find_host_by_node_id(node_id);

1090 + RRDHOST *host = rrdhost_find_by_node_id(node_id);

1091

1092 if ((!host || !(wc = host->aclk_config)))

1093 nd_log(NDLS_ACCESS, NDLP_NOTICE,

src/libnetdata/dictionary/dictionary-unittest.c

+3 -3

@@ -845,8 +845,10 @@ static void *unittest_dict_view_thread(void *arg) {

845 return arg;

846 }

847

848 -static int dictionary_unittest_view_threads() {

848 +static struct dictionary_stats stats_master = { 0 };

849 +static struct dictionary_stats stats_view = { 0 };

850

851 +static int dictionary_unittest_view_threads() {

852 struct thread_view_unittest tv = {

853 .join = 0,

854 .master = NULL,

@@ -856,8 +858,6 @@ static int dictionary_unittest_view_threads() {

858 };

859

860 // threads testing of dictionary

859 - struct dictionary_stats stats_master = {};

860 - struct dictionary_stats stats_view = {};

861 tv.master = dictionary_create_advanced(DICT_OPTION_NAME_LINK_DONT_CLONE | DICT_OPTION_DONT_OVERWRITE_VALUE, &stats_master, 0);

862 tv.view = dictionary_create_view(tv.master);

863 tv.view->stats = &stats_view;

src/libnetdata/dictionary/dictionary.c

+2 -1

@@ -595,6 +595,8 @@ void dictionary_flush(DICTIONARY *dict) {

595 ll_recursive_unlock(dict, DICTIONARY_LOCK_WRITE);

596

597 DICTIONARY_STATS_DICT_FLUSHES_PLUS1(dict);

598 +

599 + dictionary_garbage_collect(dict);

600 }

601

602 size_t dictionary_destroy(DICTIONARY *dict) {

@@ -604,7 +606,6 @@ size_t dictionary_destroy(DICTIONARY *dict) {

606

607 ll_recursive_lock(dict, DICTIONARY_LOCK_WRITE);

608

607 - dict_flag_set(dict, DICT_FLAG_DESTROYED);

609 DICTIONARY_STATS_DICT_DESTRUCTIONS_PLUS1(dict);

610

611 size_t referenced_items = dictionary_referenced_items(dict);

src/web/server/h2o/http_server.c

+1 -1

@@ -145,7 +145,7 @@ static inline int _netdata_uberhandler(h2o_req_t *req, RRDHOST **host)

145 if (!*host)

146 *host = rrdhost_find_by_guid(c_host_id);

147 if (!*host)

148 - *host = find_host_by_node_id(c_host_id);

148 + *host = rrdhost_find_by_node_id(c_host_id);

149 if (!*host) {

150 req->res.status = HTTP_RESP_BAD_REQUEST;

151 req->res.reason = "Wrong host id";

src/web/server/web_client.c

+6 -6

@@ -987,19 +987,19 @@ static inline int web_client_switch_host(RRDHOST *host, struct web_client *w, ch

987 netdata_log_debug(D_WEB_CLIENT, "%llu: Searching for host with name '%s'.", w->id, tok);

988

989 if(nodeid) {

990 - host = find_host_by_node_id(tok);

990 + host = rrdhost_find_by_node_id(tok);

991 if(!host) {

992 - host = rrdhost_find_by_hostname(tok);

992 + host = rrdhost_find_by_guid(tok);

993 if (!host)

994 - host = rrdhost_find_by_guid(tok);

994 + host = rrdhost_find_by_hostname(tok);

995 }

996 }

997 else {

998 - host = rrdhost_find_by_hostname(tok);

998 + host = rrdhost_find_by_guid(tok);

999 if(!host) {

1000 - host = rrdhost_find_by_guid(tok);

1000 + host = rrdhost_find_by_node_id(tok);

1001 if (!host)

1002 - host = find_host_by_node_id(tok);

1002 + host = rrdhost_find_by_hostname(tok);

1003 }

1004 }

1005