Skip to content

Conversation

@CReid98
Copy link

@CReid98 CReid98 commented Jun 20, 2023

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.


reload:{[date]
if[.z.w in key .rdb.reloadcalls;
.rdb.reloadcalls[.z.w]:1b;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mix of tabs & spaces on this line, surrounding lines all use just tabs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean here. From what I can see it's all just tabs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

one tab and then 8 spaces

Comment on lines +3 to +7
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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@jonathonmcmurray jonathonmcmurray Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So couple of thoughts -

  1. It should be fairly simple to do via string manipulation without using sv e.g.
`$"::",getenv[`BASEPORT],":admin:admin"
  1. 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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.proc.proctype is the current process's proctype i.e. here it will always be hdb - I don't think this is right

Comment on lines +5 to +7
$[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"];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think less duplication is needed here

Suggested change
$[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

clearinactivetime:0D01:00 // the time to keep inactive handle data

// list of process types connected for sync reload
connectedProcs:()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't use camel case anywhere else in TorQ, so let's not introduce it here please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants