fix(database): keep rrdset indexes allocated while host is archived - #23074
Merged
stelfrag merged 1 commit intoJul 9, 2026
Merged
Conversation
…ush()` to ensure safe indexing during host archival Ensure that chart indexes remain allocated for ongoing queries during archiving by introducing `rrdset_index_flush()` and delaying index destruction to `rrdhost_free_unlinked()`. Prevent potential null dereference by guarding index availability in `rrdset_find_and_acquire()`.
|
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Architecture diagram
sequenceDiagram
participant Q as Query Consumer
participant RR as rrdset_find_and_acquire()
participant H as RRDHOST
participant IDX as rrdset_index (id + name)
participant SVC as service.c (archive)
participant FREE as rrdhost_free_unlinked()
Note over Q,FREE: Host lifecycle: archived → queries → free
SVC->>H: archive host (orphan→archived)
H->>H: rrdhost_cleanup_data_collection_and_health()
H->>IDX: rrdset_index_flush()
Note over IDX: Flush name index first, then id index<br/>Indexes remain allocated, host pointers valid
IDX-->>H: indexes emptied but alive
par In-flight query before archive
Q->>RR: find chart by id
RR->>H: check host->rrdset_root_index
alt Index allocated (normal)
RR->>IDX: dictionary_get() (locked)
IDX-->>RR: chart reference
RR-->>Q: acquired chart
else Index NULL (defense-in-depth)
RR-->>Q: NULL (race with free_unlinked)
end
and Query continues during/after archive
Q->>RR: Release acquired chart
RR->>IDX: rrdset_acquired_release()
Note over IDX: dict still valid, decrements refcount<br/>Chart freed when last ref drains
IDX-->>RR: released OK
and New query after archive
Q->>RR: find chart by id
RR->>IDX: dictionary_get() (locked)
Note over IDX: Dictionary is empty, flush already ran
IDX-->>RR: NULL
RR-->>Q: NULL (chart not found)
end
Note over H,FREE: Host eventually unlinked (no new queries can reach it)
FREE->>H: rrdhost_free_unlinked()
H->>IDX: rrdset_index_destroy()
Note over IDX: Now safe to free the dictionaries<br/>No in-flight queries hold acquired charts
IDX-->>H: indexes freed
H->>H: free remaining host resources
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
vkalintiris
approved these changes
Jul 9, 2026
stelfrag
added a commit
to stelfrag/netdata
that referenced
this pull request
Jul 12, 2026
…etdata#23074) fix(database): replace `rrdset_index_destroy()` with `rrdset_index_flush()` to ensure safe indexing during host archival Ensure that chart indexes remain allocated for ongoing queries during archiving by introducing `rrdset_index_flush()` and delaying index destruction to `rrdhost_free_unlinked()`. Prevent potential null dereference by guarding index availability in `rrdset_find_and_acquire()`.
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
…23074) fix(database): replace `rrdset_index_destroy()` with `rrdset_index_flush()` to ensure safe indexing during host archival Ensure that chart indexes remain allocated for ongoing queries during archiving by introducing `rrdset_index_flush()` and delaying index destruction to `rrdhost_free_unlinked()`. Prevent potential null dereference by guarding index availability in `rrdset_find_and_acquire()`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #23056, which guarded the lookup side of the same problem.
The orphan→archived transition destroys the host's rrdset indexes and nulls
the pointers while queries may still be in flight, leaving two windows:
dictionary_destroy()defers destruction (referenced items), butrrdset_index_destroy()nullshost->rrdset_root_indexanyway, sorrdset_acquired_release()dereferences NULL. Every data query holdsacquired charts for its full duration (
query_target.c), so on parentsarchiving hosts under constant query load this eventually fires.
the pointer check and its use, and nothing holds a reference, the dict is
freed immediately and the lookup calls into freed memory.
Fix: on archive, flush the indexes (name view first, then the id index) but
keep them allocated — lookups return NULL naturally, releases always see a
valid dictionary, and a lookup racing with the archive now races with a
normal locked flush instead of destruction. The indexes are destroyed only in
rrdhost_free_unlinked(), after the host is unlinked and unreachable.Semantics are unchanged: flush is exactly what
dictionary_destroy()alreadydid internally when deferring, so charts held by queries are still freed when
the last reference drains. Host reactivation (archived→live) now reuses the
surviving index instead of recreating it (
rrdhost.calready guards initwith
if (!host->rrdset_root_index)).Summary by cubic
Prevent crashes during host archival by flushing RRD set indexes instead of destroying them. This removes a null-deref and TOCTOU window for in-flight queries and lets reactivated hosts reuse the same indexes.
rrdset_index_flush()and use it on archive; flushes name, then id index, but keeps them allocated.rrdhost_free_unlinked(), after the host is unreachable.rrdset_find_and_acquire()to avoid accessing a freed index.Written for commit 7efe32c. Summary will update on new commits.