| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Replaced SPINLOCK with SPINLOCK_TRACKED in critical database and journalfile code paths to enable deadlock detection and holder identification. Updated accompanying locking logic and introduced tracked spinlock helpers in `spinlock.h`.
There was a problem hiding this comment.
No issues found across 7 files
Confidence score: 5/5
sequenceDiagram
participant DC as Deadlock Checker (spinlock_core)
participant ST as SPINLOCK_TRACKED struct
participant DF as Datafile (users.spinlock)
participant JF as Journalfile (data_spinlock)
participant FL as Flush Thread (rrdengine.c)
Note over DC,FL: Tracked spinlock flow on contention
DF->>ST: spinlock_tracked_lock()
activate ST
ST->>DC: spinlock_lock_core (tracked=non-NULL)
activate DC
loop Spin-acquire loop
DC->>DC: Test __atomic_try_lock
alt Lock acquired
DC->>ST: spinlock_tracked_record_holder()
ST->>ST: __atomic_store holder_tid, holder_func, holder_since_ut
DC-->>DF: lock owned
DF->>DF: (critical section: acquire/release datafile refs)
DF->>ST: spinlock_tracked_unlock()
ST->>ST: __atomic_clear holder_tid
ST->>DC: spinlock_unlock (plain)
deactivate DC
deactivate ST
else Lock busy (contention)
DC->>DC: spins++
alt Every SPINS_BEFORE_DEADLOCK_CHECK iteration
DC->>ST: spinlock_tracked_deadlock_detect()
activate ST
alt Timeout exceeded (3600s)
DC->>ST: __atomic_load holder_tid, holder_func, holder_since_ut
alt holder_tid != 0
ST-->>DC: holder identity known
DC->>DC: fatal("... holder: tid=%d func='%s' held_for=%"PRIi64"s ...")
else holder_tid == 0 (stuck/corrupted)
ST-->>DC: no holder recorded
DC->>DC: fatal("... holder: NONE - possible corruption/stuck byte ...")
end
else No timeout yet
ST-->>DC: continue waiting
end
deactivate ST
end
DC->>DC: microsleep (exponential backoff)
end
end
Note over JF,DC: Journalfile parallel example (same pattern)
JF->>ST: spinlock_tracked_lock()
ST->>DC: spinlock_lock_core (tracked=non-NULL)
activate DC
DC-->>JF: lock owned
JF->>JF: (critical section: mount/unmount v2 data, update metrics)
JF->>ST: spinlock_tracked_unlock()
deactivate DC
Note over FL,DC: Flush-thread update via tracked lock
FL->>ST: spinlock_tracked_lock()
ST->>DC: spinlock_lock_core (tracked=non-NULL)
activate DC
DC-->>FL: lock owned
FL->>FL: update last_time_s on journalfile
FL->>ST: spinlock_tracked_unlock()
deactivate DC
Note over ST: Plain SPINLOCK path via spinlock_lock_with_trace<br/>passes tracked=NULL, constant-folded away<br/>— no overhead, no holder stores
Sorry, something went wrong.
- Updated SPINLOCK_TRACKED to retain holder fields after unlock for consistent deadlock diagnostics. - Added mutex-backed SPINLOCK_TRACKED implementation to align functionality across spinlock types. - Refactored and clarified comments to explain holder field handling and deadlock detection behavior.
|
Sorry, something went wrong.
@stelfrag I have started the AI code review. It will take a few minutes to complete. |
Sorry, something went wrong.
There was a problem hiding this comment.
No issues found across 7 files
Confidence score: 5/5
sequenceDiagram
participant DF as datafile.c (users.*)
participant JF as journalfile.c (data_spinlock)
participant RE as rrdengine.c (extent_flush)
participant SL as SPINLOCK (plain)
participant ST as SPINLOCK_TRACKED
participant DET as Deadlock Detector
Note over DF,RE: NEW: SPINLOCK_TRACKED replaces SPINLOCK for these locks
DF->>ST: spinlock_tracked_lock(&df->users.spinlock)
ST->>SL: spinlock_lock_core(spinlock, func, tracked)
alt Lock contested (spinning)
loop Exponential backoff + deadlock check
ST->>ST: spins++
alt spins % SPINS_BEFORE_DEADLOCK_CHECK == 0
ST->>DET: spinlock_tracked_deadlock_detect()
DET->>DET: Read holder_tid, holder_func, holder_since_ut
alt held >= SPINLOCK_DEADLOCK_TIMEOUT_SEC
DET-->>ST: fatal("DEADLOCK DETECTED [holder: tid=N func='...' held_for=N]")
end
end
ST->>ST: microsleep(usec), usec *= 2
end
end
ST->>ST: Acquired: __atomic_add_fetch(&locked, 1)
ST->>ST: spinlock_tracked_record_holder()
ST-->>DF: lock acquired
DF->>DF: Critical section (check users.available, lockers, etc.)
DF->>ST: spinlock_tracked_unlock(&df->users.spinlock)
ST->>SL: spinlock_unlock(spinlock)
Note over ST: Holder fields NOT cleared (advisory - overwritten by next acquirer)
ST-->>DF: lock released
Note over DF,RE: Same pattern for all call sites in datafile.c/journalfile.c/rrdengine.c
JF->>ST: spinlock_tracked_lock(&journalfile->data_spinlock)
ST->>SL: spinlock_lock_core(spinlock, func, tracked)
alt Contested
ST->>DET: spinlock_tracked_deadlock_detect()
DET-->>ST: Holder identity in fatal
end
ST->>ST: Record holder
ST-->>JF: lock acquired
JF->>JF: Critical section (check JOURNALFILE flags, refcount, mmap)
JF->>ST: spinlock_tracked_unlock(&journalfile->data_spinlock)
ST-->>JF: lock released
RE->>ST: spinlock_tracked_lock(&datafile->journalfile->data_spinlock)
RE->>RE: Update last_time_s
RE->>ST: spinlock_tracked_unlock(&datafile->journalfile->data_spinlock)
alt SPINLOCK_IMPL_WITH_MUTEX build
Note over ST: Mutex-backed implementation (no spin loop)
ST->>SL: spinlock_lock(&spinlock->spinlock) (calls netdata_mutex_lock)
ST->>ST: spinlock_tracked_record_holder()
ST-->>DF: lock acquired
DF->>ST: spinlock_tracked_unlock()
ST->>SL: spinlock_unlock(&spinlock->spinlock) (calls netdata_mutex_unlock)
Note over ST: holder fields recorded but no deadlock detection
end
Sorry, something went wrong.
There was a problem hiding this comment.
No issues found during runtime after hours running. LGTM!
Sorry, something went wrong.
…22725) * Switch to SPINLOCK_TRACKED for enhanced deadlock diagnostics Replaced SPINLOCK with SPINLOCK_TRACKED in critical database and journalfile code paths to enable deadlock detection and holder identification. Updated accompanying locking logic and introduced tracked spinlock helpers in `spinlock.h`. * Improve SPINLOCK_TRACKED behavior and add mutex-backed implementation - Updated SPINLOCK_TRACKED to retain holder fields after unlock for consistent deadlock diagnostics. - Added mutex-backed SPINLOCK_TRACKED implementation to align functionality across spinlock types. - Refactored and clarified comments to explain holder field handling and deadlock detection behavior. (cherry picked from commit 0daffba)
| Back | FazBrowse Home | New Git URL |
Summary
Summary by cubic
Add holder-aware spinlocks to improve deadlock diagnostics in the DB engine. Datafile and journal locks now report the blocking thread (tid, function, held-for), and tracked info persists across unlock; works under the mutex-backed build too.
New Features
Refactors
Written for commit 59055de. Summary will update on new commits.