fix streaming receiver discarding delivered data on child disconnect - #23118
Merged
Conversation
The stream thread handled poll HUP events before pending reads. Linux epoll (level-triggered) reports EPOLLIN|EPOLLRDHUP together once the peer's FIN is queued, regardless of how much delivered data remains unread, so the receiver was removed with data still in the socket buffer - discarding up to the entire in-flight payload. Measured: 221 of 3000 replicated rows survived a child that disconnected right after its final write; the rest were silently dropped. Remove the receiver immediately only on ERROR/INVALID, or on HUP without READ (EOF with nothing readable). When HUP arrives together with READ, keep the normal read path: one buffer per cycle, the same fairness as any connected child, with level-triggered polling re-reporting both flags until recv() returns 0, when the existing read-path teardown removes the receiver with the same disconnect reason as before. Replication heals such losses only for children that reconnect with local retention; ephemeral children, children without retention, and setups with replication disabled lost the delivered data permanently.
|
Merged
stelfrag
pushed a commit
to stelfrag/netdata
that referenced
this pull request
Jul 14, 2026
Ferroin
pushed a commit
that referenced
this pull request
Jul 15, 2026
ktsaou
added a commit
to ktsaou/netdata
that referenced
this pull request
Jul 21, 2026
…ngine A Go test suite that boots a completely stock netdata daemon, feeds it deterministic fixtures through the real streaming protocol (a fake child speaking the full handshake, live BEGIN2/SET2 and the replication dialogue), and verifies query results over HTTP against expectations computed from the fixture definitions - never from the engine. The suite is built as a ladder of layers, each pinning one part of the query contract before the next builds on it: - layer 0: harness self-test - byte-exact round-trips on the live path, the replication path, and across a daemon restart; - layer 1: tier0 ingestion - the edge-data palette (gap runs, resets, anomaly runs, negatives, zeros, non-default update every), the storage_number quantization contract as a Go port of pack/unpack, and single-point and beyond-retention windows; - layer 2: tier rollups - tier1/tier2 sum/min/max/count/anomaly-count derivation incl. partial and all-gap windows, float32 page rounding, and the fact that rollups aggregate original values, not quantized; - layer 3: time-aggregations - every registry time_group against exact Go ports of the grouping arithmetic, incl. options grammars, sign-dependent slot walks, running smoothing state and the incremental-sum carry; - layer 4 (parts a+b): tier fetch semantics per family over rollup tiers, and the automatic tier selection rule. MANIFEST.md tracks every case with green/red status: green cases must pass, red cases must reproduce a known bug and fail loudly when a fix lands, demanding the flip with the fixing PR. The suite found and pinned the receiver disconnect data loss (netdata#23118), the forgotten fresh-host on graceful restart (netdata#23120), and the tier query boundary absorption (netdata#23127) this way. The suite is self-contained under tests/query-corpus (go test ./... with a built netdata binary), uses no external Go dependencies, and runs in ~3.5 minutes.
ktsaou
added a commit
to ktsaou/netdata
that referenced
this pull request
Jul 21, 2026
…ngine A Go test suite that boots a completely stock netdata daemon, feeds it deterministic fixtures through the real streaming protocol (a fake child speaking the full handshake, live BEGIN2/SET2 and the replication dialogue), and verifies query results over HTTP against expectations computed from the fixture definitions - never from the engine. The suite is built as a ladder of layers, each pinning one part of the query contract before the next builds on it: - layer 0: harness self-test - byte-exact round-trips on the live path, the replication path, and across a daemon restart; - layer 1: tier0 ingestion - the edge-data palette (gap runs, resets, anomaly runs, negatives, zeros, non-default update every), the storage_number quantization contract as a Go port of pack/unpack, and single-point and beyond-retention windows; - layer 2: tier rollups - tier1/tier2 sum/min/max/count/anomaly-count derivation incl. partial and all-gap windows, float32 page rounding, and the fact that rollups aggregate original values, not quantized; - layer 3: time-aggregations - every registry time_group against exact Go ports of the grouping arithmetic, incl. options grammars, sign-dependent slot walks, running smoothing state and the incremental-sum carry; - layer 4 (parts a+b): tier fetch semantics per family over rollup tiers, and the automatic tier selection rule. MANIFEST.md tracks every case with green/red status: green cases must pass, red cases must reproduce a known bug and fail loudly when a fix lands, demanding the flip with the fixing PR. The suite found and pinned the receiver disconnect data loss (netdata#23118), the forgotten fresh-host on graceful restart (netdata#23120), and the tier query boundary absorption (netdata#23127) this way. The suite is self-contained under tests/query-corpus (go test ./... with a built netdata binary), uses no external Go dependencies, and runs in ~3.5 minutes.
ktsaou
added a commit
to ktsaou/netdata
that referenced
this pull request
Jul 23, 2026
…ngine A Go test suite that boots a completely stock netdata daemon, feeds it deterministic fixtures through the real streaming protocol (a fake child speaking the full handshake, live BEGIN2/SET2 and the replication dialogue), and verifies query results over HTTP against expectations computed from the fixture definitions - never from the engine. The suite is built as a ladder of layers, each pinning one part of the query contract before the next builds on it: - layer 0: harness self-test - byte-exact round-trips on the live path, the replication path, and across a daemon restart; - layer 1: tier0 ingestion - the edge-data palette (gap runs, resets, anomaly runs, negatives, zeros, non-default update every), the storage_number quantization contract as a Go port of pack/unpack, and single-point and beyond-retention windows; - layer 2: tier rollups - tier1/tier2 sum/min/max/count/anomaly-count derivation incl. partial and all-gap windows, float32 page rounding, and the fact that rollups aggregate original values, not quantized; - layer 3: time-aggregations - every registry time_group against exact Go ports of the grouping arithmetic, incl. options grammars, sign-dependent slot walks, running smoothing state and the incremental-sum carry; - layer 4 (parts a+b): tier fetch semantics per family over rollup tiers, and the automatic tier selection rule. MANIFEST.md tracks every case with green/red status: green cases must pass, red cases must reproduce a known bug and fail loudly when a fix lands, demanding the flip with the fixing PR. The suite found and pinned the receiver disconnect data loss (netdata#23118), the forgotten fresh-host on graceful restart (netdata#23120), and the tier query boundary absorption (netdata#23127) this way. The suite is self-contained under tests/query-corpus (go test ./... with a built netdata binary), uses no external Go dependencies, and runs in ~3.5 minutes.
ktsaou
added a commit
to ktsaou/netdata
that referenced
this pull request
Jul 23, 2026
…ngine A Go test suite that boots a completely stock netdata daemon, feeds it deterministic fixtures through the real streaming protocol (a fake child speaking the full handshake, live BEGIN2/SET2 and the replication dialogue), and verifies query results over HTTP against expectations computed from the fixture definitions - never from the engine. The suite is built as a ladder of layers, each pinning one part of the query contract before the next builds on it: - layer 0: harness self-test - byte-exact round-trips on the live path, the replication path, and across a daemon restart; - layer 1: tier0 ingestion - the edge-data palette (gap runs, resets, anomaly runs, negatives, zeros, non-default update every), the storage_number quantization contract as a Go port of pack/unpack, and single-point and beyond-retention windows; - layer 2: tier rollups - tier1/tier2 sum/min/max/count/anomaly-count derivation incl. partial and all-gap windows, float32 page rounding, and the fact that rollups aggregate original values, not quantized; - layer 3: time-aggregations - every registry time_group against exact Go ports of the grouping arithmetic, incl. options grammars, sign-dependent slot walks, running smoothing state and the incremental-sum carry; - layer 4 (parts a+b): tier fetch semantics per family over rollup tiers, and the automatic tier selection rule. MANIFEST.md tracks every case with green/red status: green cases must pass, red cases must reproduce a known bug and fail loudly when a fix lands, demanding the flip with the fixing PR. The suite found and pinned the receiver disconnect data loss (netdata#23118), the forgotten fresh-host on graceful restart (netdata#23120), and the tier query boundary absorption (netdata#23127) this way. The suite is self-contained under tests/query-corpus (go test ./... with a built netdata binary), uses no external Go dependencies, and runs in ~3.5 minutes.
ktsaou
added a commit
to ktsaou/netdata
that referenced
this pull request
Jul 24, 2026
…ngine A Go test suite that boots a completely stock netdata daemon, feeds it deterministic fixtures through the real streaming protocol (a fake child speaking the full handshake, live BEGIN2/SET2 and the replication dialogue), and verifies query results over HTTP against expectations computed from the fixture definitions - never from the engine. The suite is built as a ladder of layers, each pinning one part of the query contract before the next builds on it: - layer 0: harness self-test - byte-exact round-trips on the live path, the replication path, and across a daemon restart; - layer 1: tier0 ingestion - the edge-data palette (gap runs, resets, anomaly runs, negatives, zeros, non-default update every), the storage_number quantization contract as a Go port of pack/unpack, and single-point and beyond-retention windows; - layer 2: tier rollups - tier1/tier2 sum/min/max/count/anomaly-count derivation incl. partial and all-gap windows, float32 page rounding, and the fact that rollups aggregate original values, not quantized; - layer 3: time-aggregations - every registry time_group against exact Go ports of the grouping arithmetic, incl. options grammars, sign-dependent slot walks, running smoothing state and the incremental-sum carry; - layer 4 (parts a+b): tier fetch semantics per family over rollup tiers, and the automatic tier selection rule. MANIFEST.md tracks every case with green/red status: green cases must pass, red cases must reproduce a known bug and fail loudly when a fix lands, demanding the flip with the fixing PR. The suite found and pinned the receiver disconnect data loss (netdata#23118), the forgotten fresh-host on graceful restart (netdata#23120), and the tier query boundary absorption (netdata#23127) this way. The suite is self-contained under tests/query-corpus (go test ./... with a built netdata binary), uses no external Go dependencies, and runs in ~3.5 minutes.
1 task
ktsaou
added a commit
to ktsaou/netdata
that referenced
this pull request
Aug 11, 2026
…ngine A Go test suite that boots a completely stock netdata daemon, feeds it deterministic fixtures through the real streaming protocol (a fake child speaking the full handshake, live BEGIN2/SET2 and the replication dialogue), and verifies query results over HTTP against expectations computed from the fixture definitions - never from the engine. The suite is built as a ladder of layers, each pinning one part of the query contract before the next builds on it: - layer 0: harness self-test - byte-exact round-trips on the live path, the replication path, and across a daemon restart; - layer 1: tier0 ingestion - the edge-data palette (gap runs, resets, anomaly runs, negatives, zeros, non-default update every), the storage_number quantization contract as a Go port of pack/unpack, and single-point and beyond-retention windows; - layer 2: tier rollups - tier1/tier2 sum/min/max/count/anomaly-count derivation incl. partial and all-gap windows, float32 page rounding, and the fact that rollups aggregate original values, not quantized; - layer 3: time-aggregations - every registry time_group against exact Go ports of the grouping arithmetic, incl. options grammars, sign-dependent slot walks, running smoothing state and the incremental-sum carry; - layer 4 (parts a+b): tier fetch semantics per family over rollup tiers, and the automatic tier selection rule. MANIFEST.md tracks every case with green/red status: green cases must pass, red cases must reproduce a known bug and fail loudly when a fix lands, demanding the flip with the fixing PR. The suite found and pinned the receiver disconnect data loss (netdata#23118), the forgotten fresh-host on graceful restart (netdata#23120), and the tier query boundary absorption (netdata#23127) this way. The suite is self-contained under tests/query-corpus (go test ./... with a built netdata binary), uses no external Go dependencies, and runs in ~3.5 minutes.
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
1 participant
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
When a streaming child disconnects immediately after writing — a crash, a restart, a short-lived connection, or completing replication and moving on — the parent discards data TCP has already delivered to it.
stream_receive_process_poll_events()removes the receiver onND_POLL_HUPbefore processingND_POLL_READ. Linux epoll (level-triggered innd-poll.c) reportsEPOLLIN|EPOLLRDHUPtogether from the moment the peer's FIN is queued — regardless of how much delivered data is still unread — so the teardown runs with data pending in the socket buffer, and that data is silently dropped.Measured with a protocol-level fake child pushing a 3000-row replication burst and closing right after the final
REND: only 221 rows were stored. The receiver's own accounting shows exactly one read buffer consumed (bytes_in= chart metadata + 15487 bytes = onePLUGINSD_LINE_MAXread); strace shows the nextepoll_waitreturningEPOLLIN|EPOLLRDHUPand no furtherrecvfromon the socket. Bursts smaller than one read buffer lose a timing-dependent amount (zero to everything); anything larger loses deterministically.Replication heals this loss only when the child reconnects with local retention. Ephemeral children (k8s pods, autoscaled VMs), children running
[db] mode = noneor with small retention, and setups with replication disabled lose the delivered data permanently — and every child restart leaves a parent-side gap until the reconnect happens.Fix
Remove the receiver immediately only on
ND_POLL_ERROR/ND_POLL_INVALID(real socket errors — on RST the kernel has already dropped the receive queue), or onND_POLL_HUPwithoutND_POLL_READ(EOF with nothing readable). When HUP arrives together with READ, run the normal read path: one buffer per poll cycle — the same fairness any connected child gets — while level-triggered polling keeps re-reporting both flags; when the buffer is drained,recv()returns 0 and the existing read-path teardown removes the receiver with the same disconnect reason as before.No new state, no drain loop, no protocol or configuration change. SSL connections terminate the same way:
SSL_ERROR_ZERO_RETURNmaps to 0 (clean close) and transport EOF maps to -1 (SSL_ERROR_SYSCALL), so the drain cannot spin.Validation
End-to-end against a stock daemon, with a fake child speaking the real streaming protocol at a fixed epoch:
BEGIN2/SET2burst + immediate close: before 723/30000 stored, after 30000/30000.RBEGIN/RSET/REND) burst + immediate close: before 3716/30000, after 30000/30000.bytes_in= metadata + full payload after the fix (210245 = 178 + 210067), for every iteration.REPLAY_CHARTreply hits a dead socket) leaves the daemon healthy and serving; a 30-cycle connect/burst/immediate-close soak shows no crash and no stuck receivers.Notes
stream-sender.c) has the same HUP-before-read ordering; its inbound traffic is parent control messages. Left untouched here pending evidence of impact.Summary by cubic
Prevents data loss in the stream receiver when a child disconnects right after writing by draining readable data before teardown. We now keep reading on HUP if data is pending, so delivered rows are not dropped.
ND_POLL_ERROR/ND_POLL_INVALID, or onND_POLL_HUPwithoutND_POLL_READ.ND_POLL_HUParrives withND_POLL_READ, continue the normal read path untilrecv()returns 0, then tear down.Written for commit a4e763c. Summary will update on new commits.