Feature heartbeat - #122
Open
alowrydi wants to merge 6 commits into
Open
Conversation
…s on Olly's PR initial implementation, integration layer rewritten for current di.* contracts and conventions.
…sher-side subscriber cleanup
DIReview Summary1 critical | 7 warning(s) | 0 suggestion(s)
|
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.
Extracts TorQ's
code/common/heartbeat.q(the.hbnamespace), di.heartbeatPublishes a periodic liveness beat over pub/sub so downstream monitors can detect that a process has stalled or blocked even while its connection is still valid, and carries the monitor role in the same module: subscribing to other processes' beats, tracking the last one seen per process, and raising warning/error transitions when they stop arriving.
Builds on an initial implementation from PR #109 (Olly, June 2026, closed unmerged when he left the project) - the domain model (state machine, tolerance-multiplier grace periods, mode-2 timer scheduling with the catch-up-storm reasoning, single-arg
init) carried through largely as designed. The integration layer is rewritten against contracts that didn't exist as merged code at the time it was written -di.log,di.handlers,di.config,di.apiall merged in early August, after #109 was closed - and two further review passes since then found and fixed a blocking-availability hazard and a stale-handle bug in the async rewrite itself, detailed below.Trello ticket - https://trello.com/c/093fht6Z/95-kdb-x-heartbeat
Files created
di/heartbeat/init.qheartbeat.q, defines export of 11 functionsdi/heartbeat/heartbeat.qdi/heartbeat/heartbeat.mddi/heartbeat/VERSIONinitand resolved bydi.depcheckdi/heartbeat/deps.qdi/heartbeat/test.csvdi/heartbeat/test.qdi/heartbeat/test_integration.csvHow to test
469/469 unit tests passing, 0 failures.
The integration suite needs a real second process and is a separate file, so
moduletest(hardcoded totest.csv) doesn't pick it up:50/50 integration tests passing, 0 failures. Exercises what a mock can't reach: the remote-subscribe handshake (
.z.wresolving correctly inside an inbound async call), the root schema tabledi.pubsubdiscovers, a published row actually crossing the wire into the monitor's store, a genuinely stalled peer not blocking the sweep, and - wired against realdi.log/di.timer/di.handlers/di.pubsubrather than mocks - the realdi.timerscheduler actually firing a registered job, not just holding it in a registry.qlint: 0 error-level findings.
Design decisions
1. The remote subscribe is asynchronous, not synchronous like legacy. A sync call blocks for as long as the peer takes to answer, and a peer that's alive but stalled - a GC pause, a heavy query, exactly the condition this module exists to detect - never answers promptly. Because the subscribe runs inside a
di.timerjob on a single thread, that block stalledpublishheartbeatandcheckheartbeattoo: the monitor fell silent to its own monitors at precisely the moment a peer misbehaved. Measured on real KDB-X: 10s blocked against a 10s-hung peer, versus 36µs for the async send.hopen's timeout doesn't help here - it bounds connection establishment, not later requests on an already-open handle.2. There is no cache of subscribed handles. An earlier version tracked which handles it had already subscribed, to skip them on later sweeps. Removed rather than repaired: kdb+ reissues the lowest free descriptor immediately (measured: handle
4→ close → reopen → handle4), andhclosedoesn't fire.z.pcat all (also measured), so a stale cache entry made the monitor skip a live peer permanently, with nothing in any log to explain it.di.pubsubalready dedupes subscribers by.z.w, so the cache bought nothing. Removing it also removed this module's only reason to register.z.pc, and with it thehandlersdependency entirely.3. Async can't report a remote failure, so two checks watch the consequence instead. A subscribed peer that never beats, and a monitor that discovers no usable peer at all. Both warn once at a configurable sweep threshold, distinguish a cold start from genuine peer loss, and close out with a matching-severity recovery line so an alert can never dangle unresolved in a log.
4. A pre-existing root
heartbeattable is never adopted, not even a column-identical one.initerrors instead. This is a real, reachable collision, not a hypothetical:di.subscriptionsinstalls subscribed tickerplant schemas at root, so a monitor watching a TP that carriesheartbeatalready has one. Column compatibility is deliberately not treated as a safety signal - a matching shape isn't evidence a table means the same thing, and adopting on that basis would silently co-mingle this module's liveness rows with whatever the real owner stores there.5. Every timer entry point that can throw is isolated against
di.timer'sdisableonfail:1bdefault. An unprotected throw in a client callback, a transient pub/sub outage, or a dependency error wouldn't just skip one beat - it would permanently disable the job, silently ending the exact monitoring this module exists to provide.onwarning/onerror/publishand the whole discovery sweep run isolated; state is always updated before an isolated callback fires, so a callback failure can never leave the store inconsistent.6. Config defaults take the values TorQ actually shipped, not the fallbacks written inside
heartbeat.qitself. Legacy carried two disagreeing sets - the in-file@[value;...]defaults (1.5f,2f,()) and whatconfig/settings/default.qactually set (2f,3f,`ALL). This module takes the shipped values, since they reflect what really ran; the in-fileconnections:()in particular could not have been used at all; it makes`ALL in ()false and the entire monitor path silently monitors nothing.Checklist
consistency.mdandstyle.mdheartbeat.mddocuments all exported functions, config, usage examples and notesdi.*modules - standalone (deps.qempty)Documentation
See
heartbeat.mdfor full reference including the dependency table, configuration options with shipped-vs-in-file defaults noted per key, exported function documentation with examples, the root-publication and remote-subscribe mechanics, and a dedicated section on deliberate departures from legacy.ection on deliberate departures from legacy.