Sync reload #577
Sync reload #577
Conversation
- dictionary expectedreloadcalls updated by .z.po and .z.pc - .z.pc automatically triggers reload if wdb or rdb disconnects - handling added for user to bypass check if reload triggered manually
-added unit tests
code/processes/rdb.q
Outdated
| @@ -114,6 +114,10 @@ endofday:{[date;processdata] | |||
| }; | |||
|
|
|||
| reload:{[date] | |||
| if[.z.w in key .rdb.reloadcalls; | |||
| .rdb.reloadcalls[.z.w]:1b; | |||
There was a problem hiding this comment.
mix of tabs & spaces on this line, surrounding lines all use just tabs
There was a problem hiding this comment.
I'm not sure what you mean here. From what I can see it's all just tabs
There was a problem hiding this comment.
one tab and then 8 spaces
| before,0,0,q,hdbHandle:hopen`::41804:admin:admin,1,,"Get handle to the HDB" | ||
| before,0,0,q,rdb1Handle:hopen`::41802:admin:admin,1,,"Get handle to RDB1" | ||
| before,0,0,q,rdb2Handle:hopen`::41803:admin:admin,1,,"Get handle to RDB2" | ||
| before,0,0,q,wdbHandle:hopen`::41805:admin:admin,1,,"Get handle to WDB" | ||
| before,0,0,q,gwHandle:hopen`::41806:admin:admin,1,,"Get handle to gateway" |
There was a problem hiding this comment.
hardcoded ports in unit tests isn't ideal - a future developer should be able to run all unit tests in TorQ easily, without having to faff about with configuring base ports or worrying about port collisions etc.
There was a problem hiding this comment.
I'm having some trouble resolving this issue. I've tried getting the baseport from the system variable and then joining it with sv, but this gives `::/41800/:admin:admin. I'm not sure where to go from here.
There was a problem hiding this comment.
So couple of thoughts -
- It should be fairly simple to do via string manipulation without using
sve.g.
`$"::",getenv[`BASEPORT],":admin:admin"- TorQ has functions for getting connections to other processes based on proctype/procname without manually constructing connection strings etc. - if we use those, we shouldn't have to worry about any of this
There was a problem hiding this comment.
I think more clear comments are needed in this file - it's not at all clear why numbers of connections should be what they are etc., so it's very unclear what is actually being tested here
tests/syncreload/eod.csv
Outdated
There was a problem hiding this comment.
as with the other test CSV, more clear comments needed - not clear why rdb1 & hdb should be updated after calling .u.end on the wdb, but not rdb2 & gw?
| reloadcalls:()!(); | ||
|
|
||
| // function to add handle to reloadcalls dictionary | ||
| po:{[h] if[.proc.proctype in .hdb.connectedProcs;reloadcalls[h]:0b]}; |
There was a problem hiding this comment.
.proc.proctype is the current process's proctype i.e. here it will always be hdb - I don't think this is right
| $[not all .hdb.reloadcalls; | ||
| {.lg.o[`reload;"reload call received from handle ", string[.z.w], "; reload calls pending from handles ", ", "sv string where not .hdb.reloadcalls]; :(::)}[]; | ||
| .lg.o[`reload;"reload call received from handle ", string[.z.w], "; no more reload calls pending"]; |
There was a problem hiding this comment.
I think less duplication is needed here
| $[not all .hdb.reloadcalls; | |
| {.lg.o[`reload;"reload call received from handle ", string[.z.w], "; reload calls pending from handles ", ", "sv string where not .hdb.reloadcalls]; :(::)}[]; | |
| .lg.o[`reload;"reload call received from handle ", string[.z.w], "; no more reload calls pending"]; | |
| .lg.o[`reload;"reload call received from handle ", string[.z.w], $[a:not all .hdb.reloadcalls;"; reload calls pending from handles ", ", "sv string where not .hdb.reloadcalls;""]]; | |
| // return early if there are still pending calls | |
| if[a;:(::)]; |
or something like this
| @@ -9,6 +9,9 @@ querykeeptime:0D00:30 // the time to keep queries in the | |||
| errorprefix:"error: " // the prefix for clients to look for in error strings | |||
| clearinactivetime:0D01:00 // the time to keep inactive handle data | |||
|
|
|||
| // list of process types connected for sync reload | |||
| connectedProcs:() | |||
There was a problem hiding this comment.
we don't use camel case anywhere else in TorQ, so let's not introduce it here please
Synchronise reloads in a system where there are multiple subscribers and one hdb, such that the hdb will not reload until all subscribers have completed their reload process.