fix(aclk): prevent unbounded one-core CPU spins in the cloud-connection loops - #22879
Merged
Conversation
…on loops Two independent ACLK loops could peg one CPU core at 100% indefinitely (until a manual restart) while the cloud link still looked healthy: 1) ACLKSYNC dispatch loop (sqlite_aclk.c): if the pending_queries counter ever drifts above the actual number of queued queries, the NOOP -> ACLK_QUERY_EXECUTE rewrite fires on every iteration over an empty queue and the inner UV_RUN_NOWAIT loop never blocks, producing a silent, work-free spin that cannot self-recover. Reconcile the counter against JudyLCount() (the source of truth) before it can gate the rewrite, and log the discrepancy (rate-limited) so the trigger is captured if it ever recurs. 2) mqtt_wss_service() and the https_client.c helper loops: when poll() keeps reporting readiness but no bytes/frames make progress, the only escapes were gated behind poll()==0 and therefore never fired. Add a per-client no-progress watchdog (drop and reconnect after 2*PING_TIMEOUT) plus a terminal socket-revents check (POLLERR/POLLNVAL) to mqtt_wss_service(); POLLHUP intentionally falls through to SSL_read so a graceful close drains its final frame. Evaluate the request timeout on every iteration of the three https_client.c poll loops. All changes are additive and inert on healthy paths, single-threaded where they touch state, and platform-independent.
ktsaou
requested review from
stelfrag,
thiagoftsm and
vkalintiris
as code owners
June 26, 2026 22:45
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 2/5
- In
src/database/sqlite/sqlite_aclk.c, the reconcile logic skips the zero-counter case, so a drifted state where the queue has work butpending_queries == 0is never corrected; merging as-is risks permanently stalled ACLK query processing until an unrelated enqueue happens. Add a recovery path for this state (or trigger reconcile when queue depth and counter disagree) and verify with a targeted test before merging.
Architecture diagram
sequenceDiagram
participant Agent as Agent Main
participant ACLK as ACLK Sync Loop
participant JudyL as JudyL Queue
participant MQTT as MQTT WSS Client
participant HTTPS as HTTPS Client
participant Cloud as Cloud Endpoint
participant poll as poll() syscall
Note over Agent,Cloud: ACLK Sync Loop – Query Dispatch (no-progress spin fix)
loop Each iteration
ACLK->>JudyL: dequeue next opcode
alt opcode == ACLK_DATABASE_NOOP and pending_queries > 0
ACLK->>JudyL: NEW: reconcile pending_queries with JudyLCount()
alt pending_queries != JudyLCount()
ACLK->>ACLK: NEW: log rate-limited warning, set pending_queries = JudyLCount()
end
ACLK->>ACLK: if pending_queries > 0 and queries_running < limit → rewrite to ACLK_QUERY_EXECUTE
else opcode == ACLK_QUERY_EXECUTE
ACLK->>JudyL: process query, decrement pending_queries
end
end
Note over MQTT,Cloud: MQTT WSS Service – I/O Handling (watchdog + error check)
MQTT->>poll: poll() with timeout
alt poll() returns 0 (clean timeout)
MQTT->>MQTT: NEW: update last_io_progress_ut
MQTT->>MQTT: handle keepalive
else poll() returns >0 (readiness)
Note over MQTT: NEW: no-progress watchdog (only for established connection)
MQTT->>MQTT: check revents for POLLERR | POLLNVAL
alt revents matches error
MQTT->>MQTT: NEW: return MQTT_WSS_ERR_CONN_DROP (break spin)
else revents not error
MQTT->>MQTT: check if no progress for MQTT_WSS_IO_WATCHDOG_SECS
alt watchdog elapsed
MQTT->>MQTT: NEW: return MQTT_WSS_ERR_CONN_DROP (break spin)
end
MQTT->>Cloud: attempt SSL read/write
alt bytes moved
MQTT->>MQTT: NEW: update last_io_progress_ut
else SSL error
MQTT->>MQTT: existing error handling
end
end
end
Note over HTTPS,Cloud: HTTPS Client Poll Loops (timeout check on every iteration)
HTTPS->>HTTPS: enter write or read loop (socket_write_all / ssl_write_all / read_parse_response)
loop Until I/O complete or error
HTTPS->>HTTPS: NEW: check https_req_check_timedout()
alt timed out
HTTPS->>HTTPS: NEW: return timeout (break unbounded spin)
else not timed out
HTTPS->>poll: poll() with timeout
alt poll() > 0 (ready)
HTTPS->>Cloud: perform underlying I/O (send/recv)
alt I/O progressed
HTTPS->>Cloud: continue loop
else I/O error
HTTPS->>HTTPS: existing error handling
end
else poll() == 0 (timeout)
HTTPS->>HTTPS: existing timeout handling (now also covered by top check)
end
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…n > 0 The reconcile only ran when pending_queries > 0, so it healed an over-count (the CPU spin) but not an under-count: if the counter drifted to 0 while aclk_query_execute->JudyL still held queued queries, the NOOP -> ACLK_QUERY_EXECUTE rewrite never fired and those queries stalled until an unrelated enqueue nudged execution. Run the reconcile on every idle (NOOP) pass so drift in either direction self-heals (JudyLCount is O(1) on an empty/small array).
Member
Author
|
@cubic-dev-ai please review again |
Contributor
@ktsaou I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
No issues found across 3 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant SyncLoop as ACLK Sync Event Loop
participant Judy as JudyL Query Queue
participant Counter as pending_queries
participant MQTT as MQTT WSS Client
participant Poll as poll/SSL
participant HTTPS as HTTPS Client
participant Timeout as Request Timeout
Note over SyncLoop,Judy: ACLK Sync Dispatch Loop (sqlite_aclk.c)
loop Each iteration
SyncLoop->>Counter: Read pending_queries
alt opcode == ACLK_DATABASE_NOOP
SyncLoop->>Judy: NEW: JudyLCount() – source of truth
Judy-->>SyncLoop: queued count
alt queued != pending_queries
SyncLoop->>SyncLoop: NEW: log discrepancy, set pending_queries = queued
end
end
alt pending_queries > 0 and slots available
SyncLoop->>SyncLoop: Rewrite NOOP → ACLK_QUERY_EXECUTE
SyncLoop->>Judy: Pop and dispatch query
end
end
Note over MQTT,Poll: MQTT WSS Service (mqtt_wss_client.c)
MQTT->>MQTT: On CONNACK – NEW: seed last_io_progress_ut
loop mqtt_wss_service entered periodically
MQTT->>Poll: poll(timeout_ms)
alt poll returns 0
MQTT->>MQTT: NEW: update last_io_progress_ut (clean idle)
MQTT->>MQTT: Handle keepalive if needed
else poll returns >0
MQTT->>MQTT: NEW: check revents & POLLERR|POLLNVAL
alt error present
MQTT->>MQTT: Drop connection, return MQTT_WSS_ERR_CONN_DROP
else
MQTT->>MQTT: NEW: check watchdog (elapsed > 2*PING_TIMEOUT?)
alt time exceeded
MQTT->>MQTT: Drop connection, return MQTT_WSS_ERR_CONN_DROP
else
MQTT->>Poll: SSL_read / SSL_write
alt bytes transferred
MQTT->>MQTT: NEW: update last_io_progress_ut
end
end
end
end
end
Note over HTTPS,Timeout: HTTPS Client Poll Loops (https_client.c)
loop socket_write_all / ssl_write_all / read_parse_response
HTTPS->>Timeout: NEW: https_req_check_timedout() on every iteration
alt timed out
HTTPS->>HTTPS: Return error (2 or HTTPS_CLIENT_RESP_TIMEOUT)
else
HTTPS->>Poll: poll(POLL_TO_MS)
alt poll > 0
HTTPS->>Poll: Read/write data
else poll error
HTTPS->>HTTPS: Handle poll error
end
end
end
thiagoftsm
approved these changes
Jun 27, 2026
thiagoftsm
left a comment
Contributor
There was a problem hiding this comment.
No issues found during runtime. LGTM!
|
stelfrag
pushed a commit
to stelfrag/netdata
that referenced
this pull request
Jul 12, 2026
Merged
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
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.



Problem
Two independent ACLK loops can peg one CPU core at 100% indefinitely (until a
manual restart) while the cloud link still looks healthy. Both were diagnosed on a
standalone Windows Server agent.
1. ACLKSYNC dispatch loop (
sqlite_aclk.c)aclk_synchronization_event_loop()keeps apending_queriescounter as an alias forthe number of queries held in
aclk_query_execute->JudyL. If that counter ever driftsabove the actual queue length, the
ACLK_DATABASE_NOOP -> ACLK_QUERY_EXECUTErewritefires on every iteration over an empty queue, and the inner
uv_run(UV_RUN_NOWAIT)loop never blocks — a silent, work-free 100% CPU spin that cannot self-recover (the
spinning thread is even accounted idle, because
worker_is_busy()excludesACLK_QUERY_EXECUTE).Fix: before the rewrite can use it, reconcile
pending_queriesagainstJudyLCount()(the source of truth) and log the discrepancy (rate-limited) if theydisagree. Inert when they match; self-heals any drift regardless of its origin.
2.
mqtt_wss_service()and thehttps_client.cpoll loopsWhen
poll()keeps reporting readiness but no bytes/frames make progress (e.g. aruntime
poll()/SSL readiness quirk), the only escapes were gated behindpoll() == 0and therefore never fired, so the loop spins.
Fix:
mqtt_wss_service(): add a per-client no-progress watchdog (drop + reconnect after2 * PING_TIMEOUT, refreshed on a cleanpoll()timeout and on every byte moved,seeded at CONNACK, gated to an established connection), plus a terminal
socket-
reventscheck onPOLLERR | POLLNVAL.POLLHUPintentionally falls throughto
SSL_readso a graceful close drains its final frame before the cleanSSL_ERROR_ZERO_RETURN.https_client.c: evaluate the existing request-timeout check at the top of all threepoll loops (
socket_write_all,ssl_write_all,read_parse_response), not only onpoll() == 0.Notes
state, and platform-independent (no Windows-only special-casing).
memory/Judy corruption on the MSYS2/Cygwin runtime); the reconcile is defense-in-depth
that ends the spin for any cause and logs the condition so a recurrence is captured.
Summary by cubic
Prevents unbounded one-core CPU spins and stalled ACLK queues by fixing two cloud-connection loops. The agent now self-recovers instead of burning CPU when poll reports readiness without progress.
pending_querieswithJudyLCount()before any rewrite; rate-limit a warning and self-correct drift to prevent both spins (over-count) and stalled queues (under-count).mqtt_wss_service(): add a per-client no-progress watchdog (drop/reconnect after2 * PING_TIMEOUT) and drop onPOLLERR | POLLNVAL;POLLHUPstill drains viaSSL_read.https_client.c: check the overall request timeout on every iteration insocket_write_all,ssl_write_all, andread_parse_responseto break spin states.Written for commit 975b281. Summary will update on new commits.