@cryptotaxi247 / netdata-1 / commits / 4c1da82e9

Improve datafile deletion process (#21781)

* - Introduce `pending_deletion` flag to better manage deletion lifecycle. - Add retry mechanism with a maximum attempt limit for acquiring datafiles for deletion. - Enhance synchronization handling for writers and lockers to prevent race conditions. - Log detailed tracing for open cache eviction and deletion phase. - Prevent adding open-cache pages for datafiles marked for deletion. * initializes `ret` to false and remove redundan else branches

Stelios Fragkakis committed Feb 19, 2026 at 18:38 UTC 4c1da82e9a22e14efa58cd73146a99d551ac76a8
4 files changed +143 -77
src/database/engine/datafile.c
+119 -74
@@ -47,6 +47,7 @@ static struct rrdengine_datafile *datafile_alloc_and_init(struct rrdengine_insta
47 datafile->magic1 = datafile->magic2 = DATAFILE_MAGIC;
48
49 datafile->users.available = true;
50 + datafile->users.pending_deletion = false;
51
52 spinlock_init(&datafile->users.spinlock);
53 spinlock_init(&datafile->writers.spinlock);
@@ -55,18 +56,36 @@ static struct rrdengine_datafile *datafile_alloc_and_init(struct rrdengine_insta
56 return datafile;
57 }
58
58 -ALWAYS_INLINE bool datafile_acquire(struct rrdengine_datafile *df, DATAFILE_ACQUIRE_REASONS reason) {
59 - bool ret;
59 +ALWAYS_INLINE bool datafile_acquire(struct rrdengine_datafile *df, DATAFILE_ACQUIRE_REASONS reason)
60 +{
61 + bool ret = false;
62
63 spinlock_lock(&df->users.spinlock);
64
63 - if(df->users.available) {
64 - ret = true;
65 - df->users.lockers++;
66 - df->users.lockers_by_reason[reason]++;
65 + if(df->users.available && reason < DATAFILE_ACQUIRE_MAX) {
66 + bool allow = true;
67 +
68 + if(df->users.pending_deletion) {
69 + if(reason == DATAFILE_ACQUIRE_OPEN_CACHE) {
70 + // Hold writers.spinlock to read the writer counters, ensuring proper
71 + // memory ordering on weakly-ordered architectures (ARM) - consistent
72 + // with all other read sites in the codebase.
73 + spinlock_lock(&df->writers.spinlock);
74 + size_t writers_running = df->writers.running;
75 + size_t flushed_to_open_running = df->writers.flushed_to_open_running;
76 + spinlock_unlock(&df->writers.spinlock);
77 + allow = (writers_running || flushed_to_open_running);
78 + }
79 + else
80 + allow = false;
81 + }
82 +
83 + if(allow) {
84 + ret = true;
85 + df->users.lockers++;
86 + df->users.lockers_by_reason[reason]++;
87 + }
88 }
68 - else
69 - ret = false;
89
90 spinlock_unlock(&df->users.spinlock);
91
@@ -87,95 +106,121 @@ void datafile_release_with_trace(struct rrdengine_datafile *df, DATAFILE_ACQUIRE
106 bool datafile_acquire_for_deletion(struct rrdengine_datafile *df, bool is_shutdown)
107 {
108 bool can_be_deleted = false;
109 + bool marked_pending = false;
110 + bool should_evict_open_pages = false;
111
112 spinlock_lock(&df->users.spinlock);
113
93 - if(!df->users.lockers) {
114 + if(!df->users.pending_deletion) {
115 + df->users.pending_deletion = true;
116 + marked_pending = true;
117 + }
118 +
119 + // Hold writers.spinlock to read the writer counters, ensuring proper memory ordering
120 + // on weakly-ordered architectures (ARM). Without this, stale reads could cause premature
121 + // deletion while a writer is still active - writers use these counters (not lockers) for
122 + // their lifecycle, so lockers alone cannot protect against this race.
123 + spinlock_lock(&df->writers.spinlock);
124 + size_t writers_running = df->writers.running;
125 + size_t flushed_to_open_running = df->writers.flushed_to_open_running;
126 + spinlock_unlock(&df->writers.spinlock);
127 +
128 + if(!writers_running && !flushed_to_open_running && !df->users.lockers) {
129 can_be_deleted = true;
130 df->users.available = false;
131 }
97 - else {
98 - // there are lockers
132 + else if(df->users.lockers)
133 + should_evict_open_pages = true;
134 + spinlock_unlock(&df->users.spinlock);
135 +
136 + if(marked_pending)
137 + netdata_log_info("DBENGINE: datafile %u of tier %d is pending deletion (%s)",
138 + df->fileno, datafile_ctx(df)->config.tier, is_shutdown ? "shutdown" : "runtime");
139
100 - // evict any pages referencing this in the open cache
101 - spinlock_unlock(&df->users.spinlock);
140 + if(can_be_deleted)
141 + return true;
142 +
143 + if(should_evict_open_pages)
144 pgc_open_evict_clean_pages_of_datafile(open_cache, df);
103 - spinlock_lock(&df->users.spinlock);
145
105 - if(!df->users.lockers) {
106 - can_be_deleted = true;
146 + usec_t time_to_scan_ut = now_monotonic_usec();
147 + size_t clean_pages_in_open_cache = pgc_count_clean_pages_having_data_ptr(open_cache, (Word_t)datafile_ctx(df), df);
148 + size_t hot_pages_in_open_cache = pgc_count_hot_pages_having_data_ptr(open_cache, (Word_t)datafile_ctx(df), df);
149 + time_to_scan_ut = now_monotonic_usec() - time_to_scan_ut;
150 +
151 + spinlock_lock(&df->users.spinlock);
152 +
153 + spinlock_lock(&df->writers.spinlock);
154 + writers_running = df->writers.running;
155 + flushed_to_open_running = df->writers.flushed_to_open_running;
156 + spinlock_unlock(&df->writers.spinlock);
157 +
158 + if(!writers_running && !flushed_to_open_running) {
159 + if(df->users.available) {
160 df->users.available = false;
161 + netdata_log_info("DBENGINE: datafile %u of tier %d entered deletion phase-2 (new users blocked)",
162 + df->fileno, datafile_ctx(df)->config.tier);
163 }
109 - else {
110 - // there are lockers still
111 -
112 - // count the number of pages referencing this in the open cache
113 - spinlock_unlock(&df->users.spinlock);
114 - usec_t time_to_scan_ut = now_monotonic_usec();
115 - size_t clean_pages_in_open_cache = pgc_count_clean_pages_having_data_ptr(open_cache, (Word_t)datafile_ctx(df), df);
116 - size_t hot_pages_in_open_cache = pgc_count_hot_pages_having_data_ptr(open_cache, (Word_t)datafile_ctx(df), df);
117 - time_to_scan_ut = now_monotonic_usec() - time_to_scan_ut;
118 - spinlock_lock(&df->users.spinlock);
119 -
120 - if(!df->users.lockers) {
121 - can_be_deleted = true;
122 - df->users.available = false;
123 - }
164
125 - else if(!clean_pages_in_open_cache && !hot_pages_in_open_cache) {
126 - // no pages in the open cache related to this datafile
127 -
128 - time_t now_s = now_monotonic_sec();
129 -
130 - if(!df->users.time_to_evict) {
131 - // first time we did the above
132 - df->users.time_to_evict = now_s + (is_shutdown ? DATAFILE_DELETE_TIMEOUT_SHORT : DATAFILE_DELETE_TIMEOUT_LONG);
133 - internal_error(true, "DBENGINE: datafile %u of tier %d is not used by any open cache pages, "
134 - "but it has %u lockers (oc:%u, pd:%u), "
135 - "%zu clean and %zu hot open cache pages "
136 - "- will be deleted shortly "
137 - "(scanned open cache in %"PRIu64" usecs)",
138 - df->fileno, datafile_ctx(df)->config.tier,
139 - df->users.lockers,
140 - df->users.lockers_by_reason[DATAFILE_ACQUIRE_OPEN_CACHE],
141 - df->users.lockers_by_reason[DATAFILE_ACQUIRE_PAGE_DETAILS],
142 - clean_pages_in_open_cache,
143 - hot_pages_in_open_cache,
144 - time_to_scan_ut);
145 - }
165 + if(!df->users.lockers)
166 + can_be_deleted = true;
167
147 - else if(now_s > df->users.time_to_evict) {
148 - // time expired, lets remove it
149 - can_be_deleted = true;
150 - df->users.available = false;
151 - internal_error(true, "DBENGINE: datafile %u of tier %d is not used by any open cache pages, "
152 - "but it has %u lockers (oc:%u, pd:%u), "
153 - "%zu clean and %zu hot open cache pages "
154 - "- will be deleted now "
155 - "(scanned open cache in %"PRIu64" usecs)",
156 - df->fileno, datafile_ctx(df)->config.tier,
157 - df->users.lockers,
158 - df->users.lockers_by_reason[DATAFILE_ACQUIRE_OPEN_CACHE],
159 - df->users.lockers_by_reason[DATAFILE_ACQUIRE_PAGE_DETAILS],
160 - clean_pages_in_open_cache,
161 - hot_pages_in_open_cache,
162 - time_to_scan_ut);
163 - }
168 + else if(!clean_pages_in_open_cache && !hot_pages_in_open_cache) {
169 + time_t now_s = now_monotonic_sec();
170 +
171 + if(!df->users.time_to_evict) {
172 + df->users.time_to_evict = now_s + (is_shutdown ? DATAFILE_DELETE_TIMEOUT_SHORT : DATAFILE_DELETE_TIMEOUT_LONG);
173 + internal_error(true, "DBENGINE: datafile %u of tier %d pending deletion has %u lockers "
174 + "(oc:%u, pd:%u, rt:%u, ix:%u), writers %zu/%zu, open-cache clean/hot %zu/%zu "
175 + "- will force-delete shortly (scanned in %"PRIu64" usecs)",
176 + df->fileno, datafile_ctx(df)->config.tier,
177 + df->users.lockers,
178 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_OPEN_CACHE],
179 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_PAGE_DETAILS],
180 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_RETENTION],
181 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_INDEXING],
182 + writers_running,
183 + flushed_to_open_running,
184 + clean_pages_in_open_cache,
185 + hot_pages_in_open_cache,
186 + time_to_scan_ut);
187 }
165 - else
166 - internal_error(true, "DBENGINE: datafile %u of tier %d "
167 - "has %u lockers (oc:%u, pd:%u), "
168 - "%zu clean and %zu hot open cache pages "
169 - "(scanned open cache in %"PRIu64" usecs)",
188 + else if(now_s > df->users.time_to_evict) {
189 + can_be_deleted = true;
190 + internal_error(true, "DBENGINE: datafile %u of tier %d pending deletion has %u lockers "
191 + "(oc:%u, pd:%u, rt:%u, ix:%u), writers %zu/%zu, open-cache clean/hot %zu/%zu "
192 + "- forcing delete now (scanned in %"PRIu64" usecs)",
193 df->fileno, datafile_ctx(df)->config.tier,
194 df->users.lockers,
195 df->users.lockers_by_reason[DATAFILE_ACQUIRE_OPEN_CACHE],
196 df->users.lockers_by_reason[DATAFILE_ACQUIRE_PAGE_DETAILS],
197 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_RETENTION],
198 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_INDEXING],
199 + writers_running,
200 + flushed_to_open_running,
201 clean_pages_in_open_cache,
202 hot_pages_in_open_cache,
203 time_to_scan_ut);
204 + }
205 }
206 }
207 +
208 + if(!can_be_deleted)
209 + internal_error(true, "DBENGINE: datafile %u of tier %d pending deletion has %u lockers "
210 + "(oc:%u, pd:%u, rt:%u, ix:%u), writers %zu/%zu, open-cache clean/hot %zu/%zu "
211 + "(scanned in %"PRIu64" usecs)",
212 + df->fileno, datafile_ctx(df)->config.tier,
213 + df->users.lockers,
214 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_OPEN_CACHE],
215 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_PAGE_DETAILS],
216 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_RETENTION],
217 + df->users.lockers_by_reason[DATAFILE_ACQUIRE_INDEXING],
218 + writers_running,
219 + flushed_to_open_running,
220 + clean_pages_in_open_cache,
221 + hot_pages_in_open_cache,
222 + time_to_scan_ut);
223 +
224 spinlock_unlock(&df->users.spinlock);
225
226 return can_be_deleted;
src/database/engine/datafile.h
+2 -1
@@ -79,6 +79,7 @@ struct rrdengine_datafile {
79 unsigned lockers;
80 unsigned lockers_by_reason[DATAFILE_ACQUIRE_MAX];
81 bool available;
82 + bool pending_deletion;
83 time_t time_to_evict;
84 } users;
85
@@ -122,4 +123,4 @@ static struct rrdengine_instance *datafile_ctx(struct rrdengine_datafile *datafi
123 void sync_uv_file_data(uv_file file);
124 #endif
125
125 -#endif /* NETDATA_DATAFILE_H */
\ No newline at end of file
126 +#endif /* NETDATA_DATAFILE_H */
src/database/engine/pagecache.c
+7 -2
@@ -1067,8 +1067,13 @@ void pgc_open_add_hot_page(
1067 unsigned extent_size)
1068 {
1069
1070 - if(!datafile_acquire(datafile, DATAFILE_ACQUIRE_OPEN_CACHE)) // for open cache item
1071 - fatal("DBENGINE: cannot acquire datafile to put page in open cache");
1070 + if(!datafile_acquire(datafile, DATAFILE_ACQUIRE_OPEN_CACHE)) { // for open cache item
1071 + nd_log_limit_static_thread_var(erl, 10, 0);
1072 + nd_log_limit(&erl, NDLS_DAEMON, NDLP_INFO,
1073 + "DBENGINE: skipped adding open-cache page for datafile %u of tier %u (deletion in progress)",
1074 + datafile->fileno, datafile->tier);
1075 + return;
1076 + }
1077
1078 struct extent_io_data ext_io_data = {
1079 .fileno = datafile->fileno,
src/database/engine/rrdengine.c
+15
@@ -1478,6 +1478,7 @@ void datafile_delete(
1478 worker_is_busy(UV_EVENT_DBENGINE_DATAFILE_DELETE_WAIT);
1479
1480 bool datafile_got_for_deletion = datafile_acquire_for_deletion(datafile, false);
1481 + size_t attempts = 0;
1482
1483 while (!datafile_got_for_deletion) {
1484 if(worker)
@@ -1486,6 +1487,20 @@ void datafile_delete(
1487 datafile_got_for_deletion = datafile_acquire_for_deletion(datafile, false);
1488
1489 if (!datafile_got_for_deletion) {
1490 + if(++attempts >= 30) {
1491 + // pending_deletion is already set, blocking new acquires.
1492 + // Bail out and let the next rotation cycle retry - lockers
1493 + // will drain over time since no new ones can be added.
1494 + netdata_log_error("DBENGINE: datafile %u of tier %d could not be acquired for deletion "
1495 + "after %zu attempts (%u lockers remain) - will retry on next rotation",
1496 + datafile->fileno, ctx->config.tier, attempts, datafile->users.lockers);
1497 +
1498 + if(worker)
1499 + worker_is_idle();
1500 +
1501 + return;
1502 + }
1503 +
1504 netdata_log_info("DBENGINE: waiting for data file '%s/"
1505 DATAFILE_PREFIX RRDENG_FILE_NUMBER_PRINT_TMPL DATAFILE_EXTENSION
1506 "' to be available for deletion, "