Feature server - #120
Conversation
…ill mocked awaiting merge
DIReview Summary0 critical | 6 warning(s) | 0 suggestion(s)
|
DIReview Summary0 critical | 1 warning(s) | 0 suggestion(s)
|
…ore getservers ALL/null contract to couple correctly with di.heartbeat and prevent self-connections
DIReview Summary0 critical | 3 warning(s) | 0 suggestion(s)
|
…r/handler deps at init Every exported function except getapimeta now refuses to run before init has completed. Previously a pre-init call failed silently rather than loudly: getservers returned an empty table, gethandlebytype 0Ni, and waitfortype spun its full timeout before returning 0b - all three indistinguishable from "genuinely nothing of that type is connected". Only startup threw, and then with a bare unnamed error off the unset connections config. The probe reads the injected logger, which has no load-time default; self and registered are both declared at load and so cannot tell "init ran" from "module loaded". teardown releases the two process-global registrations init installs - the .z.pc handler and the serversretry timer job - and resets the registered flag so a later init re-registers rather than being skipped by the idempotency guard. Module state, SERVERS included, is deliberately left intact so a shutdown path can still inspect what was connected. Idempotent: di.timer.deletejobs is a delete-where and di.handlers' removesimple early-returns, so a second call cannot die on what the first removed. init now checks that the timer dep exposes a callable deletejobs, and the handlers dep a callable register and remove. A dict hands back a null for a missing key, so an unvalidated dep turns teardown's @[f;ids;handler] from protected apply into three-argument amend: it upserts into a throwaway dict, deletes nothing, warns nothing, and teardown still reports success. The callable test excludes 101h (::) as well as testing the 100 112h range, because :: sits inside that range while being callable in no useful sense and is exactly what a function-valued dep dict returns for a missing key. Verified end to end - without the exclusion a null deletejobs passed init and left serversretry scheduled after teardown. Test fixtures resolve from the module directory via .Q.m.mp rather than the current working directory, so the suite runs from outside the repo root. Suite is 93 checks, all passing. Each new guard and check was confirmed by deletion: removing any one of them fails its own rows and no others.
| / reset the SIDE-EFFECT guard only, so a later init registers both again. initialised[] probes | ||
| / .z.m.loginfo, not this, so everything else stays callable after a teardown - see initialised[] | ||
| .z.m.registered:0b; | ||
| .z.m.loginfo[`teardown;"di.servers .z.pc registration and serversretry job removed"]; |
There was a problem hiding this comment.
teardown updates registered via .z.m.registered:0b but reads registered (the module-level variable) in init's idempotency guard. If .z.m is the module namespace and these are the same slot this is fine, but if registered in init resolves as a local/different binding the reset will not be seen and the idempotency guard will never re-allow registration after a teardown. Verify that every read of registered inside init goes through .z.m.registered (or the same slot) so the teardown reset is actually observed.
| if[99h<>type deps[`timer]`addjob; | ||
| '"di.servers: timer`addjob must be a variant dict (see di.timer addjob.custom/default/simple)"]; | ||
| if[not `custom in key deps[`timer]`addjob; | ||
| '"di.servers: timer`addjob must expose the `custom variant [id;func;params;period;mode;opts]"]; |
There was a problem hiding this comment.
The closing ] for the if[not deletejobs in key depstimer; ...] block is missing — the two-line string literal forming the error message is never closed with ]. This leaves the subsequent if[99h<>type depshandlers;...]parsed as part of the firstifbody, so the handlers type check is only evaluated whendeletejobsis absent. A timer dep that hasdeletejobsbut passes a non-dicthandlersvalue will not be caught. Add the missing]after the"di.timer"]string to close the firstif` before opening the next.
| {[ev;ph;nm] handlercalls::delete from handlercalls where event=ev,name=nm;}; | ||
| {[ev]}); | ||
|
|
||
| warnlogged:{[s] any (exec msg from logrows where lvl=`warn) like "*",s,"*"}; |
There was a problem hiding this comment.
mockhandlersremove uses a global amend (handlercalls::delete from handlercalls where event=ev,name=nm). In a module-namespaced context the bare name handlercallson the right-hand side of:: resolves to root (.handlercalls), but the table was defined at the top of this file without a namespace qualifier. If the test file is loaded inside a namespace (as .Q.m.mp/ k4unit may do), the global assign will write to a different slot than the oneregisterupserts into, so teardown assertions that count rows inhandlercallswill always see the pre-teardown count and the 'removed' assertion will pass vacuously. Use an explicit path (e.g..t.handlercalls`) for both the table definition and all references to it.
DIReview Summary0 critical | 3 warning(s) | 0 suggestion(s)
|
3 participants
di.servers — TorQ Modularisation PR
Summary
Extracts TorQ's
.serversconnection management (code/handlers/trackservers.q+servers.q) into a standalone kdb-x module:di.servers. The module dials a staticprocess.csv"phone book", maintains a pool of live handles to peer processes, hands them out by proctype via a selection algorithm, and recovers dropped connections. It satisfies the di.* module contract: one-arginit[deps]dependency injection, strict validation with no fallback, a conservative exported API,getapimetafordi.api, and no harddi.*dependencies (log,timerandhandlersare all injected).Background
TorQ's
.serverstracks connected backend processes and lets a gateway (or any process) look up a handle to a peer by type. In TorQ this was entangled with the discovery service, password/access-list files, non-TorQ process tracking, environment reads, and the global process framework.This PR is part of the broader TorQ → kdb-x modularisation effort.
di.serversis a FRAMEWORK-tier module scoped down for v1: no discovery service, no password/access-list files, no non-TorQ process tracking, no FinSpace, and no environment reads.process.csvis treated as a static phone book (who to dial), not an identity source — a process's self-identity arrives via config, injected bydi.torq. This makes connection management independently loadable, testable, and drivable from resolved config rather than TorQ globals.Changes
New files
di/servers/servers.qinit,startup,getservers,gethandlebytype,waitfortype,getapimeta, plus internal helpers (retry,cleanup,opencon,formathp,readprocesscsv,retryrows,selector,updatestats,signalfound,raiseerror)di/servers/init.qservers.qand declares the export listdi/servers/test.csvdi/servers/test.qdi/servers/servers.mdDifferences from TorQ original
.serversdi.servers.lg.o/.lg.ecallslogdependency, three flat vars (.z.m.loginfo/logwarn/logerr), binary{[c;m]}.z.pcvia.dotz.sethandlersdep;.z.pcregistered as a simple/observer event viaregister[event;phase;nm;pri;func](fan-out, side-effect only).timerframeworktimerdep;timer[`addjob][`custom]6-arg variant, a 10s (mode-1, seconds)serversretryjob.servers.SERVERStable.z.m.SERVERSmodule-local mutable stateprocess.csv/ discoveryproctype/procnameindeps);process.csvis a dial-only phone bookprocess.csvpath arrives viaconfigprocesscsv` (resolved by di.torq)process.csvparsinghost,port,proctype,procname4-column layout; header validated and rejected loudly (positional read would otherwise misparse silently)startupdepcycleswaitfortype[proctype;timeoutms;pollms]— "fail fast, but wait for a hard dependency"; caller decides if timeout is fatalusesingleton, strictinit[deps](no fallback),raiseerrorlog-then-signal,export:list,getapimetaExported API
init/getapimetaare exported as plumbing (di.torq calls them by convention) but are deliberately not listed ingetapimeta[]— the registry describes the callable API, not plumbing.deps(injectables + config, assembled by di.torq)inittakes a singledepsdict carrying both the injectable dependencies and this process's resolved config slice (the one-arg convention shared withdi.config):log`info`warn`error{[c;m]}logger dict —di.logsatisfies it directlytimerdi.timerexport; callstimer[`addjob][`custom](6-arg variant)handlersdi.handlerscontract;register[event;phase;nm;pri;func]proctype/procnameprocess.csvconnections.tomlcascade — normalised). Default: noneprocesscsvprocess.csv. Required only onceconnectionsis non-emptyinitwires the deps, records self-identity, and installs two one-time process-global side effects — the.z.pccleanup observer and the 10s retry job — guarded by an internalregisteredflag soinitis idempotent (a duplicatedi.timer.addjobid would otherwise throw).initdoes not open connections; that isstartup's job.process.csv format (v1)
Strict 4-column layout — header validated up front and rejected loudly if reordered or wider:
startupreads this phone book, drops this process's own row (matched onproctype+procname), connects to every row whose proctype is inconnections, and records each as aSERVERSrow. A failed connect is logged, not raised, and left asw:0Niforretryto reopen.startupis idempotent — it skips procs already tracked, so a repeat call (or a grownprocess.csv) adds only new rows, never a duplicate or a leaked second handle.Connection recovery
.z.pcobserver marks the closed handle's row disconnected (w:0Ni,endpstamped).serversretryjob (10s) runscleanupto sweep handles that vanished fromkey .z.Wwithout a clean.z.pc, then reopens every dead (null) handle viaretry.waitfortypereusesretryto block at startup until a hard-dependency proctype comes up (or a timeout elapses).Test coverage
Tests are in
test.csvand run viak4unit(the live-handle tests spin up real peer processes viatest.q):Areas covered
init— dependency + config validationlog/timer/handlers, non-dict / partiallog, badtimershape (missingaddjob/customvariant), non-dicthandlers, missing/non-symbolproctype/procname— all with the"di.servers"error prefix (plain signal; logger not wired yet)init— wiring and idempotencyinitrefreshes refs without re-registering the.z.pchandler or the retry jobstartup— live + dead peergethandlebytype— live remote handlecleanup+retrykey .z.W)waitfortype1b; timeout case returns0bstartupidempotencyreadprocesscsvfail-loudgetapimetagetapimeta[]lists exactly the callable exports (plumbing omitted) with the registry columnsdi.logintegrationdi.loglogger itself, not just the recording mockAll green in local KDB-X runs (live-peer suite).
Notes
di.servershas no harddi.*module dependencies —log,timerandhandlersare injected and all required;initsignals immediately (plain', logger not yet wired) if any is missing.formathpbuilds a`:host:porthandle); a futureSOCKETTYPEconfig would reintroducetcps/unixwhen there is a real requirement and a test — no unexercised branches ship.hopenuses the single 2-item timeout formhopen (handle;timeoutms)(the dyadic form throws'rank); defaultHOPENTIMEOUTis 2000ms.10000would have been ~2.8h, the latent typo that made dead-handle recovery never fire in early POCs.SERVERSis mutated as a source-level.z.m.SERVERS(catenate-and-reassign) so it picks up the compile-time module-local rewrite; a symbol-based`.z.m.SERVERS`insert would silently target the wrong table.di.torqowns centraldi.apiregistration of thegetapimeta[]rows.